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.
