CDV ❯ Bad use of WeakHashMaps in ThreadID stuff

• Type: Bug
• Status: Closed
• Priority: 2 Major
• Resolution: Fixed
• Component/s:
• Labels:

• Assignee: cdennis
• Reporter: teck
• Created: March 10, 2010
• Votes: 0
• Watchers: 1
• Updated:

  July 15, 2010
• Resolved:

  May 21, 2010

Description

The map used in com.tc.object.locks.ThreadIDFactory has three issue:

1) Is it really safe to be doing unsynchronized access to a WeakHashMap instance? Apart from the visibility problems, can it cause an infinite loop like HashMap can? 2) The key (an autoboxed instance of Long) will likely immediately become garbage making the map not very effective as a cache 3) For some keys (-127..128) the map will never get cleared since the Long instances are cached in Long.valueOf()

This map should likely be using weak VALUE (not keys) and needs to be thread safe

The map used in com.tc.util.runtime.ThreadIDMapJdk15 should be identity based. If someone has a subclass of Thread that doesn’t do hashCode()/equals() right it can cause an assertion. Of course there is no IdentityWeakHashMap in the JDK.

Fortunately one can use google collections and get maps that do exactly what we want here.

Comments

Chris Dennis 2010-05-21

Fixed by using google collections weak valued maps. I believe that the indication to merge this to 3.2 is a mistake, Fiona?