A patch for CycleBreaker (which secures access to _threadHash with an
ReentrantReadWriteLock) is attached to CASTOR-2746 [1].
Cheers,
Torsten
[1] http://jira.codehaus.org/browse/CASTOR-2746
On 23.06.2009, at 22:09, Torsten Juergeleit wrote:
CycleBreaker claims to be a "thread-safe detection of cyclic calls"
but it's implementation isn't thread-safe.
This is because it uses as "_threadHash" an unsynchronized instance
of IdentityHashMap.
<quote from javadoc of IdentityHashMap >
Note that this implementation is not synchronized. If multiple
threads access this map concurrently, and at least one of the
threads modifies the map structurally, it must be synchronized
externally. (A structural modification is any operation that adds or
deletes one or more mappings; merely changing the value associated
with a key that an instance already contains is not a structural
modification.) This is typically accomplished by synchronizing on
some object that naturally encapsulates the map. If no such object
exists, the map should be "wrapped" using the
Collections.synchronizedMap method.
</quote>
This seems to be a similar issue as with
XMLClassDescriptorResolverImpl [1] which needed a
ReentrantReadWriteLock as well.
I will come up with a patch.
Cheers,
Torsten
[1] http://jira.codehaus.org/browse/CASTOR-2019
On 23.06.2009, at 20:26, Godmar Back wrote:
Oh, the irony of a class called "cycle breaker" being involved in a
memory leak!
From a brief look at this class:
http://jira.codehaus.org/secure/attachment/27944/CycleBreaker.java
it seems the problem is caused by calls to "startingToCycle" not
being matched with calls to "releaseCycleHandle". You may wish to
examine the code for this situation.
- Godmar
On Tue, Jun 23, 2009 at 2:11 PM, Harry Hehl <[email protected]
> wrote:
Hello
With castor 1.2 and 1.3 if equals/hashcode generation is enabled the
generated equals() method calls
org.castor.core.util.CycleBreaker.startingToCycle .
org.castor.core.util.CycleBreaker contains private static
IdentityHashMap _threadHash = new IdentityHashMap();
During testing it was discovered the application was leaking
memory. It
was tracked to _threadHash referencing many objects when they are no
longer referenced anywhere else.
Why would an object not be removed from _threadHash? Not generating
hash/equals methods eliminates the leak, but they are required by the
application.
Can this cycle breaker code be disabled? i.e. is it really necessary?
Thanks.
