Inconsistent synchronization in GenericObjectPool; constant fields should be final

• Key: POOL-137
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Unassigned
• Reporter: Sebb
• Fix Version/s: 1.5

Description

_whenExhaustedAction is not always accessed using synchronization.

The private static Latch class appears to be used across threads, but has no synchronization.

The static class Config contains several fields which appear to be used as constants, however the fields are not final.

Comments

[Mark Thomas]

I can't sync _whenExhaustedAction where it is used or I end up with a deadlock when running the tests. I think what I'll do is get a local copy of all the fields I need at the start of borrowObject() - in a sync block - so at least we are working with a consistent configuration for the rest of that method even if it might be out of date.

For the use of Latch, I am happy (at the momentat least ) that the code is written in such a way that only one thread will ever be trying to access the latch at the same time. If you can see a code path where that isn't the case please add it here.

The Config class looks OK to me. Which fields were you worried about?

I have fixed the sync issues as best I can even though I am not a huge fan of my solution. I think the whole sync strategy needs to be re-thought for 2.0. I'll write up something for the dev list when we reach that point.

[Sebb]

Re: _whenExhaustedAction : this should be documented for later maintainers please.

Access to Latch fields need to be synch. regardless of whether the access is simultaneous or not - if one thread sets a field, another thread may not see the same value in the absence of synch.

Re: Config - I thought the fields were used as constants, but on looking again they are used as defaults - so please ignore that comment.

[Mark Thomas]

Re: _whenExhaustedAction : Already documented in-line

Re: Latch: Syncs added.

[Phil Steitz]

Fixed in 1.5 release.