Make WrapDynaClass instance cache thread-safe (1.X)#419
Conversation
Signed-off-by: Naveed Khan <dxbnaveed.k@gmail.com>
|
Hi @rootvector2 |
|
Looked into it. The synchronization here is fine; the test asserted more than the weak cache guarantees. The |
Port of #418 to the
1.Xbranch.WrapDynaClass.createDynaClassreads and populates its per-classloader cache through an unsynchronized get/create/put over a plainWeakHashMap, so threads that share a context classloader can corrupt the map and receive distinctWrapDynaClassinstances for one key. Unlike master there is no atomiccomputeIfAbsentto lean on here (WeakFastHashMapinheritsHashMap.computeIfAbsent, which bypasses its delegate map), so the port swaps the cache toBeanUtils.createCache()— the synchronized weak-keyedWeakFastHashMapthatMethodUtilsalready uses — and synchronizes the get/create/put sequence on the cache so one key yields one instance. Includes the regression test from #418: it races 16 threads oncreateDynaClassfor one bean class over 200 rounds; it fails on unmodified1.X(two distinct instances for one key) and passes with the change.Same caveat as #416: the full default goal hits one failure in
LocaleBeanificationTest.testContextClassloaderIndependenceon my machine, but it fails identically on unmodified1.Xand passes standalone, so it is pre-existing and unrelated to this change.mvn; that'smvnon the command line by itself.