https://bz.apache.org/bugzilla/show_bug.cgi?id=65424

--- Comment #3 from Stefan Bodewig <[email protected]> ---
getHelpers has always been synchronized. The main reason for this has been
performance rather than thread-safety. Back in 2000 when the class was written,
reflection was incredibly expensive and we simply wanted to avoid reflecting on
a class unnecessarily.

My initial thought was "time to finally start using something more modern than
Hashtable" and indeed at first glance doing away with synchronization and using
ConcurrentHashMap compute(IfAbsent) could work. But that may cause situations
where we run into deadlocks as well if loading the helper for a class somehow
triggers loading a helper for the same class once again. Also the "same class
different classloader" logic might need some thought.

Your patch may cause us to run reflection on the same class more often than
strictly required but I think this is acceptable today. It does lack the "is
this the same classloader" check, though, so you should add oh.bean == ih.bean
in addition to oh != null inside of the new block (and add braces, while you
are at it :-)).

Right now I cannot see any additional problems the patch could cause.

Thank you.

-- 
You are receiving this mail because:
You are the assignee for the bug.

Reply via email to