Comment 1 Andrew Eisenberg 2009-02-05 13:26:31 EST

Created attachment 124849 [details]
error message reported

Too long to attach directly.

Comment 2 Andrew Clement 2009-02-05 15:26:46 EST

This looks to be a re-entrancy problem.  Where two threads are using the same weaver instance at the same time.  This is not supported - the weaver is only threadsafe, it is not reentrant.

Under AspectJ loadtime weaving, entry to the weaver is gated on having a lock on the classloader associated with the weaver:

  synchronized (loader) {
    WeavingAdaptor weavingAdaptor = WeaverContainer.getWeaver(loader, context);
    byte[] newBytes = weavingAdaptor.weaveClass(className, bytes, false);
    return newBytes;
  }

Going in from equinox aspects we don't use this code but go straight for the weaver, we need to check on a similar lock.  Looking at a section of the stack:

 at org.aspectj.weaver.tools.WeavingAdaptor.getWovenBytes(WeavingAdaptor.java:423)
 at org.aspectj.weaver.tools.WeavingAdaptor.weaveClass(WeavingAdaptor.java:286)
 at org.eclipse.equinox.weaving.aspectj.loadtime.OSGiWeavingAdaptor.weaveClass(OSGiWeavingAdaptor.java:97)
 at org.eclipse.equinox.weaving.aspectj.WeavingService.preProcess(WeavingService.java:156)
 at org.eclipse.equinox.weaving.adaptors.AspectJAdaptor.weaveClass(AspectJAdaptor.java:235)
 at org.eclipse.equinox.weaving.hooks.AspectJHook.processClass(AspectJHook.java:126)

I'd expect some kind of lock possibly in the OSGIWeavingAdaptor or maybe in WeavingService.preProcess()

Comment 3 Andrew Clement 2009-02-05 18:53:53 EST

Ah no.... it isn't that - although I am intrigued about the lack of synchronized entry to the weaver from Equinox Aspects - does some higher level mechanism prevent the issue?

This appears to be a problem with the reweavable setting.  Currently (I honestly couldn't say why) the reweavable mode is a shared field across all weavers.  So all weaver instances share the setting which might be true or it might be false.  For compile time weaving it defaults to true and for load time weaving it defaults to false.  This used to be fine, because all weavers in Eclipse were for AJDT projects and typically all defaulted to true.  But now we are using weavers for JDT weaving and weavers for projects within the same VM.

This means the LTW kicks in first, creates a few weavers and everything is fine - if, however, the LTW of JDT is in the middle of this method:

org.aspectj.weaver.bcel.BcelClassWeaver.weave()

at the same point as a new AJDT project (and builder and thus weaver) is setup then we have a problem.

AJDT will switch ON reweaving for all weavers.  The thread doing JDT weaving will then crash with this NPE because the check on line 421 will have returned false whilst the check on line 533 will return true.

Why on earth is that flag static?

Note:
This will also damage the performance of JDT Weaving - since if it is incomplete (some codepath may not have been exercised yet) and an AJDT project comes along and switches the flag to TRUE, then all JDT weaving after that will suffer the cost of reweavable.

Comment 4 Andrew Clement 2009-02-05 19:30:56 EST

fix committed. hopefully it addresses this.

Comment 5 Andrew Eisenberg 2009-02-06 15:55:47 EST

*** Bug 262779 has been marked as a duplicate of this bug. ***