| Summary: | Race condition in JavaLangTypeToResolvedTypeConverter (potentially exposed through Spring AOP) | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] AspectJ | Reporter: | Eric Sirianni <eric.sirianni> | ||||||
| Component: | Compiler | Assignee: | aspectj inbox <aspectj-inbox> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | aclement, eric.sirianni, jvelez | ||||||
| Version: | 1.6.11 | ||||||||
| Target Milestone: | 1.6.12 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Linux | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Eric Sirianni
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. |