Mark Wielaard wrote:
Hi Robin
On Fri, 2008-12-19 at 14:20 +1100, Robin Garner wrote:
The clear() method of ThreadLocalMap leaves the map in an inconsistent
state, causing errors during thread deletion in JikesRVM. Reported as
Bug #38576 <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38576>. This
patch amends the clear() method to put the map back in a consistent but
empty state.
I assume this happens because JikesRVM still has some ThreadLocals it
accesses itself after Thread dead. Or is there a way in which this could
show up (assuming the VM itself isn't using ThreadLocals to keep state)?
Some nitpicks going over the ThreadLocalMap (nothing seems super
important and we can certainly apply your patch as is, but if you are
cleaning up anyway...):
- Maybe let the constructor now call clear()
since they now share the same code and intention.
- Why not use new Entry[0]?
- You could even make a private static noEntries = new Entry[0];
And make clear say entries = noEntries; So there is only ever one
instance in case there are lots of threads in the program.
- Using 0 would also make the test in overflow for entries.length more
natural (by testing for zero instead of one).
- If entries can never be null now the null check for oldEntries becomes
redundant.
Cheers,
Mark
Hi Mark,
While I agree with most of the above, I'm obviously not finding time to
incorporate your suggestions. Could we get the original patch into 0.98
before the release ?
cheers,
Robin
