Bug 31460 - Weaving class loader
Summary: Weaving class loader
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P2 enhancement (vote)
Target Milestone: 1.2   Edit
Assignee: Matthew Webster CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-02-10 10:35 EST by Martin Lippert CLA
Modified: 2004-04-02 07:31 EST (History)
4 users (show)

See Also:


Attachments
WeavingURLClassLoader zip file (2.78 KB, application/zip)
2003-02-10 10:38 EST, Martin Lippert CLA
no flags Details
WeavingURLClassLoader version 2 (3.01 KB, application/zip)
2003-02-24 02:57 EST, Martin Lippert CLA
no flags Details
UML diagram (1.65 KB, image/jpeg)
2004-03-03 13:07 EST, Matthew Webster CLA
no flags Details
UML diagram (27.97 KB, image/jpeg)
2004-03-03 13:10 EST, Matthew Webster CLA
no flags Details
Beta implementation (9.73 KB, application/octet-stream)
2004-03-20 13:59 EST, Matthew Webster CLA
no flags Details
Updated patch to include missing weaver files (14.97 KB, application/octet-stream)
2004-03-21 21:37 EST, Matthew Webster CLA
no flags Details
Addtional testcase missing from patch (317 bytes, application/octet-stream)
2004-04-02 06:56 EST, Matthew Webster CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Lippert CLA 2003-02-10 10:35:35 EST
Hi!

As discussed on the mailing list a few days ago I implemented a weaving class
loader to enable bytecode based weaving at class loading time. The class loader
is based on the URLClassLoader and can be used like any other URLClassLoader. In
addition to that you can add aspects to the weaving loader. These aspects got
woven into each class that is loaded. The test cases for the class loader aren't
implemented yet, I am still thinking about how to implement them. I will
contribute them later, okay?

-Martin
Comment 1 Martin Lippert CLA 2003-02-10 10:38:25 EST
Created attachment 3388 [details]
WeavingURLClassLoader zip file
Comment 2 Martin Lippert CLA 2003-02-10 10:41:54 EST
The implementation needs the changes of bug no 30794 to compile.
Comment 3 Wes Isberg CLA 2003-02-20 23:12:38 EST
This is great.

After a brief glance at the code, I have two questions: First, should the 
weaving class loader require that any aspects be defined before any class 
loading occurs?  Let's say my aspect advises calls to {Runnable}.run() and that 
I load some classes implementing Runnable before and after I register the 
aspect.  As a result, not all calls are advised.

That (to me) is an error.  As a possible counter-example, Sun's URLClassLoader 
does permit URL's to be added to the classpath, so that classes which did not 
load before a URL is added can be loaded.  But still that preserves the 
invariant that once a class is loaded, it's used throughout the associated 
namespace (lazy but final class loading (leaving aside for the moment debugger 
class unloading in recent VM's)).  

So for aspects, I believe there is an invariant the class loader must enforce: 
that any aspect in a namespace applies to all classes in the namespace -- which 
means the set of aspects must be defined before any classes are loaded.

Which brings up a second question: Should there be a classpath which the weaving 
class loader uses to load classes but not to weave them?   It seems like a 
weaving class loader could have essentially the same namespace declaration as 
the compiler in bytecode-weaving mode: an aspectpath, a classpath (for classes 
not to be woven), and an injars (for classes to be woven (leaving aside the 
difference in form between injars and classpath)).  Defining these only via 
constructor would enforce the aspect-namespace invariant.  In this scenario, 
when loading a class, the loader should first delegate upwards, and then attempt 
to load classes from the classpath (without weaving) and then from the injars 
path (with weaving).  But this means the aspect namespace invariant is broken 
for classes loaded from the weaving class loader's classpath.  Hmm.

By contrast, most people speak as if the weaving class loader should weave any 
class it loads (essentially equating injars with the classpath).  We could build 
a weaving class loader that did that, relying entirely on upwards delegation for 
resolving classes (not to be woven) on the classpath.  

But a weave-only class loader could not handle the scenario where there are 
classes which are not to be woven but which are required to weave.  For that the 
user would set up two custom class loaders: a parent class loader (say, 
URLClassLoader) to load the required-but-unwoven classes, and the weaving class 
loader that delegates upward to the parent class loader.  

On that note, the aspect-namespace invariant already *seems* broken for classes 
loaded from a parent class loader.  As a practical matter, we probably need to 
live with that since most users do not want to weave system classes (although I 
believe there a way in 1.4 to define the class used as the system class loader). 
One could try to make fine distinctions about namespace sub-hierarchies, but I 
doubt that would prevent users from making mistakes.  So parent class loaders  
result in an unfortunate but definable implementation limitation.  It's 
manageable because there's a concrete way to define it, based on the defining 
class loader of a class.

And users might want this "implementation limitation."  They might want to 
distinguish classes not to be woven for reasons of correctness or performance, 
segregating "known" correct (as unwoven) classes.  The System classes are the 
most obvious example, but it could also be required to share library classes 
across classloaders (one weaving, one not) for interoperability in systems where 
not all the classloaders are under the control of the user. 

It might even be the "worse-is-best" way to specify a namespace.  Normally it's 
best to specify any crosscutting in the source code, but in Java the way to 
specify namespaces is by arranging a class loader hierarchy, complete with the 
special behavior of any custom class loaders.  So at least we'd be doing it the 
Java way.

Back to the question of whether there should be an unwoven class path, given 
that the workaround of a parent class loader seems most understandable and 
correct, it seems that an initial version of the weaving class loader should 
weave all classes on its classpath.  The documentation should make it clear that 
it is treating the classpath like -injars input, and show a brief code example 
how to set up a parent loader for the unwoven classpath.  That way we can 
maintain the aspect-namespace invariant at least for classes loaded by the 
weaving/defining class loader.

One way to try to manage this long-term is to provide users with a factory 
method that takes a parent class loader, an (unwoven) classpath, an aspectpath, 
and a (woven) injars path and returns a WeavingClassLoader.  That way, we can 
change the implementation without affecting client code.  The initial 
implementation would set up a parent class loader with the classpath and return 
a child weaving class loader.  (The only problem with this is that the defining 
class loader of classes loaded from the classpath would not be the one returned 
by the factory.)  Subsequent implementations could have the weaving class loader 
define the unwoven classpath classes.

In any case, since the user will be living with this implementation limitation 
for any parent delegates, it would be nice if the user could request a warning 
from the weaving class loader when aspects would affect classes not woven 
because they were loaded by upwards delegation. It would also warn if the 
weaving class loader were extended to support an unwoven class path.
Comment 4 Wes Isberg CLA 2003-02-21 01:12:37 EST
Sorry, this example is wrong since we're not implementing callee-side call join 
points now:

> ...Let's say my aspect advises calls to {Runnable}.run() and that 
> I load some classes implementing Runnable 
> before and after I register the  aspect.

but you probably got the idea...
Comment 5 Martin Lippert CLA 2003-02-24 02:56:05 EST
I see the point and totally agree with it. The class loader should not load any
classes before he knows all the aspects that should be woven into them. Then the
class loader should weave any classes he loads (aside of the classes loaded by
the parent loader).
The question is how to archieve that. One possiblity (that you mentioned) is to
give all the aspects to the class loader via a constructor parameter. This is
the cleanest way from my point of view.
Unfortunately this does not work for me inside my project. The reason is that I
need to have an instance of the class loader before I can figure out what
aspects are available. Therefore I originally implemented the "addAspect" method.

Okay, here is the second try: I changed my original implementation of the class
loader following your argumentation. Now the constructors have two additional
arguments: One for the aspect URLs and a second one for additional classpath
entries which got not be loaded but are needed for the weaver to resolve types
during the weaving process.
All other methods to add URLs (with or without aspects) are private and
therefore not usable from outside. There is only one exception: The addAspect
method is protected to let subclasses refine and/or use it. This allows me to
use the WeavingURLClassLoader as it is. What do you think?

Comment 6 Martin Lippert CLA 2003-02-24 02:57:25 EST
Created attachment 3663 [details]
WeavingURLClassLoader version 2
Comment 7 Martin Lippert CLA 2003-04-27 08:10:22 EDT
Hi,

meanwhile I adapted my weaving Eclipse runtime to Eclipse 2.1 final and slightly
changed the architecture of the integration. The result is that I do no longer
need the specialized weaving class loader for it. I separated the functionality
into a bytecode transformer component interface that is called by a special
class loader if a class is loaded. There is one AspectJ-specific implementation
to that interface that directly uses the weaving functionality I formerly
inserted into the class loader.

The result is: I do no longer need this weaving class loader for the weaving
Eclipse runtime integration. Therefore I am free to skip the dependencyAdded
method, for example, to make this a real general purpose AspectJ load-time
weaving class loader.
Comment 8 Adrian Colyer CLA 2003-12-08 08:27:36 EST
moved to target = 1.2
Comment 9 Matthew Adams CLA 2004-01-28 12:35:08 EST
Any way to refactor to allow for post-build-but-pre-classload-time bytecode 
weaving?  I'd like to be able to compile with javac/jikes, then weave with an 
AspectJ enhancer, then run with no classload-time bytecode weaving.

It appears possible since you've separated the "bytecode transformer component 
interface"....thoughts?
Comment 10 Martin Lippert CLA 2004-01-28 12:42:56 EST
I think this is already possible with the standard AspectJ compiler using the
-injar option. This allows you to weave existing bytecode with aspects. Does
that match your case?
Comment 11 Matthew Webster CLA 2004-02-27 13:50:58 EST
Rules for weaving class loader:
1.	All aspects must be known to the weaver before any classes are loaded. 
Any aspects subsequently presented to the weaver should either be ignored or 
cause an exception to be thrown.
2.	Only classes defined by the class loader can be woven i.e. in a loader 
hierarchy neither classes loaded by a parent loader (which are visible) nor a 
child (which aren’t) are woven.
3.	Aspects defined by a parent weaving class loader are available for 
extension or weaving i.e. both abstract and concrete pointcuts may be used.
Comment 12 Matthew Webster CLA 2004-03-03 12:50:47 EST
The plan is to provide a command line tool "aj" that will perform load-time 
weaving using a supplied WeavingURLClassLoader and WeavingAdaptor to wrap the 
existing BcelWorld and BcelWeaver APIs. It will use CLASSPATH and ASPECTPATH 
(which will be some subset of CLASSPTH or absent of no weaving is required). 
Hierarchies of weaving loaders should be supported as described in the 
previous append.

As Wes as pointed out the new java.lang.instrument package can be exploited 
when 1.5 is supported. A new WeavingAdaptor subclass can connect to this API.

While using Martins sample WeavingURLClassLoader as a guide I am doing some 
reimplementation to avoid copyright and licensing issues with Sun source code.
Comment 13 Matthew Webster CLA 2004-03-03 13:07:17 EST
Created attachment 8311 [details]
UML diagram
Comment 14 Matthew Webster CLA 2004-03-03 13:10:43 EST
Created attachment 8312 [details]
UML diagram
Comment 15 Artur Biesiadowski CLA 2004-03-12 04:29:34 EST
Have you looked at java.lang.instrument package in JDK1.5 beta1 ? I know that 
weaving support is aimed for 1.4 compatibility, but IMHO 1.5 instrumentation is 
more powerful tool - especially because it allows to modify most core classes 
(except really basic ones). Maybe it could be done in way which would support 
both ways - explicit classloaders and jvm instrumentation, with classloader 
solution being a subset of instrumentation ?
Comment 16 Matthew Webster CLA 2004-03-12 14:13:57 EST
1.5 is still in beta and not fully supported yet by AspectJ. We will need to 
support 1.4 for a while. However I have a subclass of the WeavingAdaptor 
written for this enhancement, that can go into future versions of AspectJ, 
that can attach to existing class loaders using the ClassFileTransformer 
interface in Instrumentation class.

WRT to modifying system classes this breaks one of the rules of weaving class 
loaders discussed in this enhancement: a class loader must know about all 
aspects before loading a class. This is not possible for the bootstrap class 
loader.
Comment 17 Martin Lippert CLA 2004-03-16 07:59:19 EST
How do you take care of classes that are generated by the weaver (for example to
realize around closures)? I just struggled into that problem with by load-time
weaving implementation because I completely forgot to think of that for my first
implementation that is still attached to this bug.

I think the weaving class loader needs to add those generated classes somehow to
his class path.
Comment 18 Matthew Webster CLA 2004-03-17 12:45:01 EST
>How do you take care of classes that are generated by the weaver
I don't but have a testcase now to work with. Thanks for pointing that out. I 
believe a small change to the WeavingAdaptor will allow the class to be 
defined by the class loader.

I am having mixed success with ITDs and perthis/target() aspects. What is your 
experience.
Comment 19 Martin Lippert CLA 2004-03-17 13:10:58 EST
Hm, haven't tried perthis/target stuff yet. Tried successfully some
introductions some month ago (for example adding an interface to an existing
class). But my most exhaustive test case uses mostly advices (I currently advice
every method of the complete eclipse system at load-time, several thousand
classes, etc.)
Comment 20 Matthew Webster CLA 2004-03-19 05:20:27 EST
I now have around closure advice, including the generated weaver class, 
working along with ITDs and perthis/pertarget aspects.
Comment 21 Martin Lippert CLA 2004-03-19 05:52:03 EST
What was the problem with perthis/pertarget and some IDTs and how did you solve it?
Comment 22 Matthew Webster CLA 2004-03-19 06:34:33 EST
Adrian has restructured the BcelWeaver so "weaveWithouDump()" is no longer 
appropraite for LTW. There is a new "weave()" method which I use and it works. 
It also accounts for weaver-generated files. The WeavingAdaptor will make this 
all much easier!
Comment 23 Matthew Webster CLA 2004-03-20 13:59:20 EST
Created attachment 8728 [details]
Beta implementation

WeavingAdaptor, WeavingURLClassLoader, tests, javadoc, scripts, examples
Comment 24 Matthew Webster CLA 2004-03-21 21:37:13 EST
Created attachment 8738 [details]
Updated patch to include missing weaver files
Comment 25 Matthew Webster CLA 2004-04-02 06:56:13 EST
Created attachment 9146 [details]
Addtional testcase missing from patch

Please add to org.aspectj.ajdt.core module, run BcweaverJarMarker and check
resulting weaver/testdate/ltw-XXX.jar files into CVS.
Comment 26 Adrian Colyer CLA 2004-04-02 07:31:28 EST
Fixed by patch contributed my Matthew Webster, incorporating contributions my 
Martin Lippert also.