Bug 287426 - generated classes and multiple weaving
Summary: generated classes and multiple weaving
Status: NEW
Alias: None
Product: AspectJ
Classification: Tools
Component: LTWeaving (show other bugs)
Version: 1.6.6   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-24 07:43 EDT by Martin Lippert CLA
Modified: 2013-06-24 11:02 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Lippert CLA 2009-08-24 07:43:16 EDT
In the context of load-time weaving generated classes are explicitly defined by calling the classloader directly (via reflection) while weaving. If the same class is passed multiple times to the AspectJ weaver (when multiple load-time weavers are used and the AspectJ weaver is called from inside some of them) the same class is defined multiple times to the JVM (causing exceptions to be thrown).

I am not sure what a good solution to this could be or if it is just sufficient to ignore the exception and move on. Any thoughts highly welcome!
Comment 1 Simone Gianni CLA 2009-08-25 03:30:20 EDT
Could this be connected to https://bugs.eclipse.org/bugs/show_bug.cgi?id=262102 ?

The bug talks about inner classes, but applies also to closures (as they are inner classes). Just added the comment about closures, strange it was not already there.

Both could be resolved if the inner class name carried the name (or hash or whatever) of the aspect that is causing its generation, so that they are unique even when multiple parallel weaving are executing (be them LTW or CTW, it's the same).

As Andy pointed out in his mail, this could cause some useless classes to be defined by LTW (or present in the jars) but would eliminate this situation which can evolve in a manteinance nightmare quite easily.
Comment 2 Andrew Clement CLA 2009-09-23 20:56:04 EDT
hmmm - i hadn't been in there before.  That reflection stuff to defineClass for generated classes is horrible.

Due to the naming scheme in use right now, I'm not sure ignoring the failure is any good as two closures with the same name may be entirely different internally.  Simones suggestion of including the aspect (probably hash of the name) giving rise to the closure would, I think, mean two closure classes with the same name do really represent the same thing - then any attempt to define the class twice could be caught and deliberately ignored (perhaps with an info/warning).
Comment 3 Simone Gianni CLA 2009-09-23 21:55:21 EDT
While this is a problem in LTW, it is even greater in compile time weaving, because nobody is checking for duplicates on the classpath and giving any sort of error. 

I've been in that situation, and I can assure that looking a call to a method that lands on a completely different method is quite a mess :D .
Comment 4 Andrew Clement CLA 2009-09-23 22:48:05 EDT
i've been fixing aj bugs for a very long time and have never seen that situation, but i wont argue that it can occur. i will take a look at the other bug whilst considering this one.
Comment 5 Andrew Clement CLA 2009-09-25 13:34:26 EDT
I'm now looking at closure names like this with a change I've just made:

A$5f36c234$ajcClosure$1.class

- the name of the type containing the shadow
- the hashcode of the fully qualified aspect defining the around advice
- the string 'ajcClosure'
- a unique counter within the affected target type (almost an 'around advice shadow counter' number).

So the only new thing is the aspect name.  

However, I'd like to see a testcase for the problem scenario before fiddling in this space - does anyone want to build one?  (not an OSGI app, just some simulation of the weaver/classloader relationships that would lead to a double defineClass)
Comment 6 Andrew Clement CLA 2013-06-24 11:02:43 EDT
unsetting the target field which is currently set for something already released