Community
Participate
Working Groups
I am using the weaver API (mostly BcelWorld and BcelWeaer) in a multithreaded environment and ran into problems. I tracked them down to some static variables in the BCEL library implementation that can cause exceptions in the weaver (signatures are not parsed correctly) when executed via mulitiple threads. Maybe the weaver implementation can use the BCEL library in a threadsafe way to allow multiple threads working with the weaver API.
This would be a great contribution.
Contributing a simple solution seems to be somewhat difficult in this situation. I solved this problem in my usecase by creating a lock on my transformer object (that uses the AspectJ weaver API). This might not be a suitable solution for the AspectJ code. Therefore I am not sure how to solve this problem in an elegant way. Let me describe the problem in more detail. Maybe someone else has an idea how to solve it for the AspectJ code base itself. The problem is the BCEL class org.apache.bcel.classfile.Utility. It uses a static field named consumed_chars in several methods. This static field is changed as a side effect by some methods and causes therefore trouble if more than one thread is using this class. Therefore the best way to solve this problem is to patch this class, I think. But maybe we do not want to change an external library. I tried to find out what code of the AspectJ weaver implementation accesses (indirectly) the Utility class. This seems to be a quite complicated task. I was not able to solve this within an adequate time. In my use case (the multithreaded problems) the Utility class of the BCEL library is used as a result of org.aspectj.weaver.bcel.LazyMethodGen.pack(). Therefore it would be possible to create a static lock in this class to avoid multithreaded usage of the BCEL library. But I am not sure whether this is the only point where this problems can occur.
The same problem occurs in the BCEL class org.apache.bcel.generic.Type, when someone calls getType (same static field consumed_chars).
Created attachment 8380 [details] patch of Utility and Type class I changed the implementation of the BCEL classes Utility and Type so that they no longer use the static field consumed_chars. The modified versions of both classes are inside the attached zip. I tried the BcelWeaverModuleTests suite with this (runs fine). I also tried this patch with my load-time weaving implementation for the new osgi runtime of Eclipse (where I observed the multithreading problems) and that code runs fine, too. It seems to me that the multithreading problems are solved.
Created attachment 8381 [details] simple test cases for Utility and Type I wrote some very simple test cases for both classes to get a feeling if they still behave as before my patch. They don't yet test multithreading problems, because I have no clear idea how to test this stuff in a predictable way.
I just recognized that the next version of BCEL will also fix this issue. The current CVS head stream of BCEL does include a patch for the multithreading issues. So you can take the current BCEL cvs version of both classes or my patch to fix this bug.
Does that help ? In old AW, we were using Bcel and had to fix it that way: http://blogs.codehaus.org/people/avasseur/archives/000101_bcel_internal.html http://blogs.codehaus.org/people/avasseur/archives/000106_bcel_patch_submitted.html I don't know if it ended up in a bcel release or the one embedded in AJ since that time but might be worth looking at ?
The current CVS head version of BCEL does include your patch (or at least something similar). I am using the HEAD version of both source codes in my load-time weaving runtime to enable multithreading and it works fine. Therefore seems to be a good thing to integrate these patches into the BCEL version AJ is using from my point of view.
Created attachment 18398 [details] Patch to make BCEL threadsafe I adapted the fixes that are in the current BCEL cvs head for the BCEL version that is currently used for AspectJ. The changed files are attached to this bug. This makes my old self-implemented fixes for older version obsolete.
I have modified BCEL to remove the reliance on static info in those offending classes. I didn't use the patch directly as I wasn't sure about the use of threadlocal so I rewrote the offending methods (the methods really just wanted to return multiple values, so I used a holder object and return that instead). I've added a few tests to the bcel-builder project to test I didn't regress the functionality but they don't verify the weaver is now threadsafe. However, Martin has tried my changes and confirmed it works in his environment. I'll close the bug when a build is available.
marked as AJ5 M2...
Build available, see download page: aspectj-DEVELOPMENT-20050324155000.jar
