Community
Participate
Working Groups
Build Identifier: 1.6.11 Spinoff of https://jira.springsource.org/browse/SPR-8070 and Bug 337855 There is a race condition in JavaLangTypeToResolvedTypeConverter's use of the 'typeVariablesInProgress' Map. This can lead to fromType() returning NULL (which eventually leads to the NPE in World). } else if (aType instanceof java.lang.reflect.TypeVariable) { if (typeVariablesInProgress.get().get(aType) != null) { >>> // aType can get removed from the Map after this null check! return typeVariablesInProgress.get().get(aType); } The error results in the following stack trace: java.lang.NullPointerException: null at org.aspectj.weaver.World.resolve(World.java:278) ~[aspectjweaver.jar:1.6.11] at org.aspectj.weaver.World.resolve(World.java:218) ~[aspectjweaver.jar:1.6.11] at org.aspectj.weaver.World.resolve(World.java:253) ~[aspectjweaver.jar:1.6.11] at org.aspectj.weaver.TypeFactory.createParameterizedType(TypeFactory.java:42) ~[aspectjweaver.jar:1.6.11] at org.aspectj.weaver.reflect.JavaLangTypeToResolvedTypeConverter.fromType(JavaLangTypeToResolvedTypeConverter.java:88) ~[aspectjweaver.jar:1.6.11] I have also attached a BTrace script that shows further analysis of the problem: onFromType_Line91: RETURNING TYPE FROM MAP (typeVariablesInProgress: {T=T}) onFromType_FieldGet: typeVariablesInProgress: {T=T} onFromType_Return: fromType(T) --> null We have tested a patch that wraps the 'typeVariablesInProgress' Map in a ThreadLocal. In limited testing, this appears to have fixed the problem. I have attached the patch. Reproducible: Sometimes Steps to Reproduce: Perform type resolution on classes with parameterized/generic signatures concurrently with multiple threads. Note: I have been unable to reproduce this issue outside of our server. We see the issue with Spring and during Server initialization under heavy load.
Created attachment 191791 [details] Patch to JavaLangTypeToResolvedTypeConverter Use a ThreadLocal to avoid race condition
Created attachment 191792 [details] BTrace Script for diagnosing race condition You will need to supply your own "logger" implementation, or just replace with System.out
I'd prefer to avoid threadlocals if we can. Can we simply change it from: if (typeVariablesInProgress.get().get(aType) != null) { return typeVariablesInProgress.get().get(aType); } to TypeVariableReferenceType tvrt = typeVariablesInProgress.get(aType); if (tvrt!=null) { return tvrt; } I think I'd prefer to synchronize on it rather than threadlocal, if that doesn't work.
I actually did that first and can confirm that using the local var works. I switched to the ThreadLocal because that seemed more in line with your goal of using that Map to prevent infinite recursion.
Andy - I see this didn't make it in to 1.6.12. Is there anything you are waiting on from me? We'd appreciate if this could make it into 1.6.13 - seems like a quick fix. Thanks!
probably just slipped through the net. We haven't released 1.6.12 yet so there is scope to get this in. We have put out M1 and an M2 will be out shortly, likely a release in a couple of months.
committed the local variable variant of the change, hope that is sufficient.
At the company I work for, we have run into this issue in the the last spring before we release it to our customers. I was glad to had found out about this bug had been fixed but oh surprise, neither the target version (1.6.12) nor subsequent ones include the aforementioned fix. Is there any update on when this fix will make it into one of the versions? Thanks
Given the date of my last comment it should be in 1.7.0 - are you saying it isn't in that version? Possibly the fix doesn't actually address all situations where this can occur. Let me know what version you are trying it with. I guess I'll have to go in and double check whether the change is really in the code.
Yes, I got the latest version 1.7.1 and the code in JavaLangTypeToResolvedTypeConverter has not changed to reflect the fix mentioned in here
I think you did add a change but it is not what it is in the Attachments section ("Patch to JavaLangTypeToResolvedTypeConverter"). IMHO the attachment should be removed to avoid confusion on what was really changed.
I think as discussed in comment 3 I didn't want to do the threadlocal so went with the local variable variant instead. You are right I should remove the attachment. I just confirmed my local variable variant of the change is in the code. If you are using that changed version the stacktrace is likely to be slightly different - can you attach a new stacktrace for the problem you see?
We are testing now using 1.6.12. I will update if we experience it again. Thanks Andrew.
