Bug 352389

Comment 1 Andrew Clement 2011-07-18 16:44:50 EDT

fix committed to avoid adding the dup

Comment 2 Tuomas Kiviaho 2014-10-22 05:24:00 EDT

I'm getting "key not found in wovenClassFile" due to the fix because second time weaving doesn't insert the key due to this fix.

The problem manifested itself due to the fact BcelWeaver.processReweavableStateIfPresent requires overweaving when second time weaver doesn't have access to aspects already woven to bytecode. Otherwise it will produce REWEAVABLE_ASPECT_NOT_REGISTERED error.

I worked around the problem with a patch that replaces the WeaverState attribute instead of ignoring/duplicating it, but I'm not sure if this is the right way or not due to the validity/correctness concern described in previous comment. I can see that both weavings have taken place without any problems that I know of at this point.

Comment 3 Tuomas Kiviaho 2016-01-16 00:49:12 EST

Created attachment 259215 [details]
LazyClassGen.patch

I see that the problem that I described above still exists with 1.8.8. I guess that this might be related to discussion java.lang.RuntimeException: key not found in wovenClassFile <http://aspectj.2085585.n4.nabble.com/Aspectj-error-Internal-compiler-error-java-lang-RuntimeException-key-not-found-in-wovenClassFile-at--td4650879.html>.

Comment 4 Andrew Clement 2016-01-18 13:29:29 EST

I tried the patch and all the existing tests pass - although I recognize we don't have many for overweening. It'd be great to have the changes proposed here covered by some new overweening tests (some that fail without the patch and pass with it).  The use of removeAttribute makes me a little nervous but I haven't been into that code for a few years so my concerns may not be warranted.

Comment 5 Tuomas Kiviaho 2016-01-19 01:51:21 EST

Comment on attachment 259215 [details]
LazyClassGen.patch

It seems that patch contains a bit old code from 1.8.6 to 1.8.8. by accident. Mainly the section starting with reference to bug 352389 is relevant. Attribute adders and getter shouldn't be removed an toString should use the equals test.

Comment 6 Tuomas Kiviaho 2016-01-19 02:04:07 EST

Created attachment 259249 [details]
LazyClassGen.patch

Comment 7 Andrew Clement 2016-01-19 12:24:37 EST

When I tested the patch yesterday I only applied the relevant bit to LazyClassGen (not the getter/setter/equals changes). If you want me to apply it you will need to sign the Eclipse CLA (you can tell if you have signed it because there is a check mark against the CLA button next to your name in each comment on the bug).

However, I'd still like to have a test case. If you aren't sure how to create one as a patch, if you can at least give me a snippet of code to compile and a set of command line steps that trigger failure, I can convert that into a test case.

Comment 8 Eduard Matsukov 2016-02-22 06:26:15 EST

Created attachment 259859 [details]
ajc crash log

Hello everybody!
This error also reproduces on ajc 1.8.8 in my case:
— I've been delcared my java output as -inpath to handle weaving sub-java languages, like groovy or kotlin, which cannot be accessed in CTW, but with binary weaver only;
— This inpath contains already woven aj classes;
— Restricting inpath to concrete directory (package) doesn't allow ajc to weave anything; (below more details about this)

The aspects that breaks ajc you can find here in my example project:
https://github.com/Archinamon/AspectJExampleAndroid/tree/master/app/src/main/aspectj/com/archinamon/xpoint

Dunno, am I should crete a separate bug report for this one?
I'm trying to restrict ajc binary weaving from passing all files in my output dir, so I walk across all sources, marks aspectj folder and exclude it from inpath directories. I'm adding to inpath only not-common directories with non-aspect binary outputs (java, groovy, etc.). But this behaviour doesn't recognisable by ajc — it doesn't weave my .class files at all if inpath not equals to the common build variant output dir.
Hard to describe it, so I'll write simple example:
My java output path (android project) is: "/Project/app/build/intermediates/classes/debug/"
where 'app' is a sub-module, 'debug' is my build variant.
When I'm trying to set inpath to smth different from the path mentioned above, for example: "/Project/app/build/intermediates/classes/debug/com/archinamon/grooid/" where I have groovy compiled code, then ajc not pick up this bytecodes at all. No errors or warnings I didn't get :(

Comment 9 Andrew Clement 2019-01-21 13:46:58 EST

Just to say under https://bugs.eclipse.org/bugs/show_bug.cgi?id=389678 I just enhanced overweaving support. 1.9.3 will probably not fail in the way you had it fail in comment#8