Comment 1 Andrew Clement 2010-08-23 13:58:07 EDT

Hey

Are you absolutely 100% certain it is 1.6.10.  This was a known serious problem for AspectJ 1.6.7 (and less likely, but possible on earlier versions).

Andy

Comment 2 Abraham Nevado 2010-08-23 14:32:37 EDT

Now, not 100 % I have realized Liferay includes some aspectj libraries at the  ROOT  web application, taking into account the Tomcat class Loader Hierarchy, which prevents overriding all the classes from the bootclasspath I am not sure. 

However the stacktrace matches the source code line from aspectj 1.6.10.... 
The setup is using LTW  and the LTW aspectjweaver is 1.6.10 (100%). it could be picking some classes from the local webapp repo (it has aspectjweaver.jar + aspectj-rt.jar)  (thus using and old version of aspect) and the others from the boottclasspath?

I was trying to create some patch for debug and testing purposes and I will forward to you as soon all the tests are passed

Comment 3 Andrew Clement 2010-08-23 14:37:13 EDT

as per my email to you just now, we need to ensure the world isn't populated with wrong data, rather than just try to cope with bad data later.

Comment 4 Andrew Clement 2010-08-23 14:38:55 EDT

just for ref: bug 298908 discusses this situation.

Comment 5 Abraham Nevado 2010-08-23 15:10:00 EDT

I have removed, literally all aspect* that is not from aspect-1.6.10 from the liferay installation so now it should be using only 1.6.10.

We still get the same error. I am including the ajcores. Although we compiled the aspectj version with the assertion uncommented, the assertion does not seem to appear in the logs neither in the cores....

Comment 6 Abraham Nevado 2010-08-23 15:10:36 EDT

Created attachment 177255 [details]
Cores with assertion enabled

Comment 7 Abraham Nevado 2010-08-24 14:47:30 EDT

I believe I know who to blame, have a look at this debug stack trace:


ATCHUNG: introducing GenericType. The QUID is:
java.lang.Exception: This is the quid
        at org.aspectj.weaver.World$TypeMap.insertInExpendableMap(World.java:1168)
        at org.aspectj.weaver.World$TypeMap.demote(World.java:1090)
        at org.aspectj.weaver.World$TypeMap.demote(World.java:1059)
        at org.aspectj.weaver.World.demote(World.java:1800)
        at org.aspectj.weaver.loadtime.ClassLoaderWeavingAdaptor.accept(ClassLoaderWeavingAdaptor.java:852)
        at org.aspectj.weaver.tools.WeavingAdaptor.weaveClass(WeavingAdaptor.java:309)
        at org.aspectj.weaver.loadtime.Aj.preProcess(Aj.java:96)
        at org.aspectj.weaver.loadtime.ClassPreProcessorAgentAdapter.transform(ClassPreProcessorAgentAdapter.java:54)
        at sun.instrument.TransformerManager.transform(TransformerManager.java:169)
        at sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:365)
        at java.lang.ClassLoader.defineClass1(Native Method)
        at java.lang.ClassLoader.defineClassCond(ClassLoader.java:632)

^_^

Comment 8 Abraham Nevado 2010-08-24 14:51:57 EDT

In fact what is going on (I need to test before being sure) is that when demoting is achieved, the object is demoted to generic type:

		if (type.isParameterizedOrGenericType()()) {
							type = type.getGenericType();
						}
						List<ConcreteTypeMunger> typeMungers = type.getInterTypeMungers();
						if (typeMungers == null || typeMungers.size() == 0) {
							tMap.remove(key);
							insertInExpendableMap(key, type);
							demotionCounter++;
						}


....

Comment 9 Abraham Nevado 2010-08-24 14:58:42 EDT

Created attachment 177357 [details]
Experimental Patch

This patch fixed the issue properly. However all the test has not been performed yet. Keep updated as soon as passed.

Comment 10 Abraham Nevado 2010-08-24 16:25:33 EDT

Comment 8 includes the patched source code, the original one is:



      if (type.type.isParameterizedOrRawType()) {
                            type = type.getGenericType();
                        }
                        List<ConcreteTypeMunger> typeMungers =
type.getInterTypeMungers();
                        if (typeMungers == null || typeMungers.size() == 0) {
                            tMap.remove(key);
                            insertInExpendableMap(key, type);
                            demotionCounter++;
                        }

The point is avoiding the inclusion of  GenericTypes into TypeMap, including its explodeableMap, where before the patch all RawTypes could be demoted to GenericType, thus being the root cause of the problem... To do so just replace type.type.isParameterizedOrRawType with:  isParameterizedOrGenericType() should avoid RAW demoting conversion to GenericType....

Comment 11 Andrew Clement 2010-08-24 21:23:22 EDT

interesting find.

I propose a slight variant of the fix.  If the original code was

if (type.isParameterizedOrRawType()) {
  type = type.getGenericType();
}

this incorrectly reduced parameterized or raw to generic (as you discovered).

Your change was to make it.

if (type.isParameterized()) {
  type = type.getGenericType();
}

which addresses the raw case.  However, given that parameterized types can't go into the map at all (the put() method forbids it), we can actually remove that code entirely, because demotion will never be processing any.

So delete all 3 lines.

I'd also propose the same change to the compile time weaving demotion code that is a few lines further down and does a similar thing.  Do you think my change will also address your case?  I'm running the tests against that change now.

It is hard to test demotion reliably as behaviour of type demotion can rely on GC behaviour, but at least we can check nothing obvious breaks.

Comment 12 Abraham Nevado 2010-08-25 06:58:23 EDT

Thanks.


I was a little bit scared to change more about demotion as far as I am not familiarized enough yet with the functional requirements... Thats the reason why I just removed Generic change only for raw types...


Andy, I tested this, and as you know in our env we were always able to reproduce the issue, however after testing with this we are not able anymore.

Comment 13 Andrew Clement 2010-08-25 10:37:31 EDT

thanks for confirming the fix.  Good bit of detective work to track it down, thanks.

Comment 14 Abraham Nevado 2010-08-25 10:42:58 EDT

Thanks to all of you because of AspectJ ;)

Comment 15 Martin Cerny 2010-12-25 12:21:59 EST

Sorry to report that, but after having this bug with 1.6.7 and 1.6.9, I have just experienced this bug with AspectJ 1.6.10, while performing a Maven build under Linux.

java.lang.StackOverflowError
       at org.aspectj.weaver.UnresolvedType.forSignature(UnresolvedType.java:427)
       at org.aspectj.weaver.UnresolvedType.getRawType(UnresolvedType.java:585)
       at org.aspectj.weaver.ResolvedType.getRawType(ResolvedType.java:2331)
       at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:427)
       at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:393)
       at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:427)
       at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:393)
       at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:427)
       at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:393)

Comment 16 Martin Cerny 2010-12-25 16:56:51 EST

As for my previous report, I am unable to reproduce the bug. Although this is an automated build on a server, that starts everytime from clean SVN checkout and there has been no activity in the repository, I only got the bug once. On the bright side, it means it is not such a big trouble.

Comment 17 Martin Cerny 2011-02-06 17:19:33 EST

Created attachment 188415 [details]
Output from AspectJ compiler when the bug occurred

I managed to reproduce the bug on our Linux build server. I attach log output from AspectJ 1.6.10

Comment 18 Andrew Clement 2011-02-06 17:23:54 EST

Martin - were there any ajcore files about when the problem occurred? I'd like to have a look at them if there were. thanks.

Comment 19 Martin Cerny 2011-02-06 18:10:48 EST

(In reply to comment #18)
> Martin - were there any ajcore files about when the problem occurred? I'd like
> to have a look at them if there were. thanks.

I am sorry, I tried to rebuild the project with new revision and the problem is no longer reproducible, even though I tried reverting to the old revision of the project. The ajcore files (if there were any) were deleted with the rebuild.

Comment 20 Martin Cerny 2011-02-06 18:11:12 EST

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