Phil Steitz wrote: > Good catch. Yes, needs to by synched (and that could be tricky, with > destroy between). Looks to me like destroy destroys the pairs, but > does not clear the queues (prior to change) and clear clears _poolList > but not _poolMap. Could be I am wrong. I Will look at this some more > this eve.
I did some more digging and I think r780905 needs to be reverted. The root cause is an issue you and Sebb had already identified - the eviction cursors are not reset on clear(). Stepping through the code what happens is that the first eviction attempts to destroy an object that has already been destroyed. This appears to succeed - hence why we end up one eviction short at the end of the test. I can reproduce the problem and clearing the eviction cursors as part fo clear appears to resolve the issue. I have tried (and failed) to add some tests for this. What I really want to do have the factory destroy() method throw an exception if you try to destroy the same object twice. Unfortunately, the pool implementations just swallow this exception. Logging (ie POOL 2.x) could be a solution to this. Another option is to remove the throws declaration from the factory destroy() method and leave it up to factory implementations to determine what exceptions are safe to be swallowed. That is a small, but very significant, API change that needs discussion come 2.x I'll revert r780905, add the fix described above and than re-review the dev archive and look at the other potential issues identified over the last few days. Mark > > On 6/2/09, Mark Thomas <ma...@apache.org> wrote: >> pste...@apache.org wrote: >>> Author: psteitz >>> Date: Tue Jun 2 02:01:22 2009 >>> New Revision: 780905 >>> >>> URL: http://svn.apache.org/viewvc?rev=780905&view=rev >>> Log: >>> Ensure that clear() fully clears the pool. >> <snip/> >> >>> @@ -1293,6 +1293,7 @@ >>> } >>> } >>> destroy(toDestroy); >>> + _poolMap.clear(); >>> } >>> >>> /** >> >> Still working through the e-mails from over the weekend but on first >> inspection that _poolMap.clear() needs to be inside the sync block to >> keep it thread safe. Given that, I am having troubling seeing how the >> _poolMap isn't already empty at that point. I wonder if we got to the >> real root cause of the bug ...? >> >> I'll get set up to do some testing and report my findings. >> >> Mark >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
