Comment 1 Hermann Vosseler 2009-11-10 15:56:16 EST

Created attachment 151878 [details]
stacktrace - now with current compiler version

Managed now to use compiler version 1.6.6 from within the Maven plugin, but the problem is still there

Comment 2 Hermann Vosseler 2009-11-10 16:40:45 EST

(just for completeness) I've now generated project files with the maven eclipse plugin; now I get the same NPE (identical stacktrace) with the AJDT integrated compiler...

Comment 3 Andrew Clement 2009-11-10 17:25:12 EST

to perhaps help us get to the bottom of it.  The issue comes up when dealing with some pointcuts in an inheritance hierarchy - we are checking override rules are correct.  One of two possible things are null, not sure which just yet - either a list item in a collection of pointcuts, or the declaring type of a pointcut.

Comment 4 Hermann Vosseler 2009-11-10 17:37:33 EST

It is quite clear that the project where this happens is missing a huge pile of dependencies (we're in the middle of figuring out how to get them into this top-level EAR project -- it worked prior to MAVEnisation).

So, basically the compile is missing a fraction of our server's internal components and thus usually emits hundreds of errors. I managed now to isolate those two Java files which -- when *removed* from the build, make the NPE go away and just the normal compile proceed with spitting out those error messages.

Comment 5 Hermann Vosseler 2009-11-10 17:43:44 EST

well... those two Java classes as such are quite innocuous. BUT, there is an Aspect involved, which combines them. Alternatively, when I remove just this Aspect, the NPE goes away too

Comment 6 Hermann Vosseler 2009-11-10 17:49:33 EST

Created attachment 151893 [details]
Aspect, involed in making the NPE appear/go away

(this aspect augments an existing business facade + provides an on-demand initialisation. additionally, there is an policy (declare error)

Note: the following imports are currently missing:

import baag.boa.esn.server.systemdienst.BOAStatusService;
import baag.boa.mvc.KompaktController;
import baag.boa.mvc.RufKontext;
import baag.boa.mvc.KompaktController.Kommando;


hope this helps to shed some light....

Comment 7 Andrew Clement 2009-11-10 19:48:22 EST

Created attachment 151895 [details]
Replacement classes that include debug

I just can't trigger the problem.  I suspect it is a pointcut with a blank declaring type that ends up in a list where it shouldn't, however I can't make that happen with my testcode.   It is probably an object created for either a cflow() or an if().

I've attached a patch to apply to the compiler jar (and weaver jar if you use it) that replaces a few classes in org/aspectj/weaver.

The extra diagnostics that will come out are:

UnresolvedType t = existing.getDeclaringType();
if (t==null) {
	System.err.println("Declaring type for pointcut "+existing+" is null");
	System.err.println("accumulated = "+acc);
	System.err.println("added = "+added);
}

Are you able to apply this?  To apply just back up the compiler you have then apply it something like this:

jar -xvf <patch>
jar -uvf aspectjtools.jar org

it will either NPE on a new line (telling us it isn't the declaring type that is at fault), or tell us precisely which pointcut it failed on.

Comment 8 Hermann Vosseler 2009-11-11 12:49:55 EST

Hi... got a rather wired result: the produced stacktrace is *exactly*
identical to my previous stacktrace
(only date and adress of the org.aspectj.weaver.bcel.BcelWorld differs)

I am rather sure that maven actually used my patched aspecttools-1.6.6.jar
I run in offlline mode, and when I remove classes from this jar, then I
get a "Class not found". 


(I am out of the office tomorrow and friday, probably I won't be able
to do further tests prior to next monday)

Comment 9 Hermann Vosseler 2009-11-16 08:44:35 EST

any further tests I could do?

Comment 10 Andrew Clement 2009-11-16 12:23:12 EST

Hi Hermann,

I've actually committed some debug into HEAD:

ResolvedType pointcutDeclaringType =
   existing.getDeclaringType().resolve(getWorld());
if (pointcutDeclaringType == null) {
  throw new BCException("Pointcut declaring type is unexpectedly null.  Pointcut is " + existing.toString());
}
if (!isVisible(existing.getModifiers(), pointcutDeclaringType, this)) {

if you were able to just grab AspectJ 1.6.7 (latest dev build) and run with it, the debug should tell us at least which pointcut is giving rise to the issue - that may help me recreate.  (The latest AJDT dev builds include this change too)

Comment 11 Andrew Clement 2009-11-17 13:40:32 EST

I'd prefer not to put in a guard that blindly proceeds even if the pointcut declaring type is null - so I'll wait to hear if you are able to try a 1.6.7 build.

Comment 12 Hermann Vosseler 2009-11-18 09:54:10 EST

Created attachment 152476 [details]
stacktrace - with aspectj-DEVELOPMENT-20091116200530

Hi Andy, sorry for the delay...

now I've run the maven build with the aspectjtools.jar taken from latest dev.

Still a bit puzzles me that I can't find any diagnostics. Where are they supposed to come out? Does at least the line numers indicate for you that there *should* be diagnostics output somewhere?

Maven is a complicated beast when it comes to debugging things (sigh)

Comment 13 Hermann Vosseler 2009-11-18 10:14:50 EST

Created attachment 152480 [details]
stdout/stderr from direct compiler run

...just to be sure, I've now extracted the compiler invocation command and classpath from Maven's debug log and re-run it directly without maven by invoking Main from aspecttools jar. The produced AJCore shows no significant differences in this case (only that we're not invoked from maven code etc.)

Comment 14 Andrew Clement 2009-11-18 12:14:13 EST

urgh, I rushed that debug and messed it up.  Sorry Hermann.  I've committed one more change, this time the message will be like this:

System.err.println("DEBUG>>> Pointcut declaring type is unexpectedly null.  Pointcut is " + existing.toString());

so it will come out to System.err.  However, this time rather than crashing at that point, it will now continue processing.  That will also let us know if a guard is sufficient in this case - I honestly don't know if it will be as I don't understand how a null declaring type pointcut is getting processed by that code.

This won't be in a dev build until later today.

Comment 15 Hermann Vosseler 2009-11-18 13:53:40 EST

Hi Andy,

> This won't be in a dev build until later today.

do you have any idea if "later" will happen during the next 3 hours? Because I won't be in office tomorrow and friday. Alternatively, you might mail me (or post here) just the changed classes.

But, anyway, we have found a way to "live" with this NPE currently, thus my primary interest is to help getting it fixed generally at some point in the furture; thus don't bother. If not today, I'll run the test next week....

Cheers,
Hermann

Comment 16 Andrew Clement 2009-11-18 14:15:46 EST

Created attachment 152503 [details]
replacement class with debug and guard

Here is the replacement class for aspectjtools.jar (and weaver if you are also using that jar, but probably not if you are outside of eclipse).

Apply like the earlier one.

This includes the debug per my previous comment.

Comment 17 Hermann Vosseler 2009-11-18 15:20:58 EST

oops... probably I'll need also the inner classes from ResolvedType in the current version?


(NoSuchMethodError) org.aspectj.weaver.ResolvedType$MethodGetter.<init>(Lorg/aspectj/weaver/ResolvedType$MethodGetter;)V
org.aspectj.weaver.ResolvedType$MethodGetter.<init>(Lorg/aspectj/weaver/ResolvedType$MethodGetter;)V
java.lang.NoSuchMethodError: org.aspectj.weaver.ResolvedType$MethodGetter.<init>(Lorg/aspectj/weaver/ResolvedType$MethodGetter;)V
        at org.aspectj.weaver.ResolvedType.<clinit>(ResolvedType.java:281)
        at org.aspectj.weaver.World.<init>(World.java:165)

Comment 18 Andrew Clement 2009-11-18 15:29:24 EST

Created attachment 152509 [details]
patch

ewww....  I should perhaps have said, that is a patch for AspectJ 1.6.7, not for 1.6.6 - were you trying to patch 1.6.6?  I was just giving you a patch of what is in the next 1.6.7 dev build (so kind of the delta from the current dev build).

The inner types haven't changed for the last few days so I thought they would be fine.  However, just in case you were patching 1.6.7 and it wasn't working, here is a new patch that includes those files too.  This won't work patched into 1.6.6, too much else has changed in ResolvedType in order to get the weaver speedups I've blogged about.

Comment 19 Hermann Vosseler 2009-11-18 16:14:58 EST

no worry, I did use the latest published dev build (1.6.7) as base for patching
And indeed, when adding the other inner classes, the compile goes ahead

but, strange enough, I get again a NPE but no diagnostics.
Just the line numbers changed slightly

--- ajcore.unpatched.txt	2009-11-18 22:14:48.000000000 +0100
+++ ajcore.patched.txt	2009-11-18 22:14:56.000000000 +0100
@@ -1,14 +1,14 @@
 ---- AspectJ Properties ---
 AspectJ Compiler DEVELOPMENT built on Tuesday Nov 17, 2009 at 01:18:33 GMT
 ---- Dump Properties ---
-Dump file: ajcore.20091118.221308.761.txt
+Dump file: ajcore.20091118.220854.631.txt
 Dump reason: java.lang.NullPointerException
 Dump on exception: true
 Dump at exit condition: abort
 ---- Exception Information ---
 java.lang.NullPointerException
-	at org.aspectj.weaver.ResolvedType.addPointcutsResolvingConflicts(ResolvedType.java:2273)
-	at org.aspectj.weaver.ResolvedType.getExposedPointcuts(ResolvedType.java:2247)
+	at org.aspectj.weaver.ResolvedType.addPointcutsResolvingConflicts(ResolvedType.java:2275)
+	at org.aspectj.weaver.ResolvedType.getExposedPointcuts(ResolvedType.java:2249)
 	at org.aspectj.ajdt.internal.compiler.lookup.EclipseSourceType.checkPointcutDeclarations(EclipseSourceType.java:439)
 	at org.aspectj.ajdt.internal.compiler.lookup.AjLookupEnvironment.resolvePointcutDeclarations(AjLookupEnvironment.java:467)
 	at org.aspectj.ajdt.internal.compiler.lookup.AjLookupEnvironment.completeTypeBindings(AjLookupEnvironment.java:262)
@@ -208,8 +208,8 @@



Just to be sure, I'll try again to extract the commandline and run it without Maven....

Comment 20 Hermann Vosseler 2009-11-18 16:25:22 EST

Created attachment 152524 [details]
stdout/stderr from direct compiler run (2 - with patch-2)

OK, here is the result from issuing the compiler standalone (command line extracted from maven debug log).

There is one interesting line: "null" immeditately before the second stacktrace. Is this the diagnostics?

Comment 21 Andrew Clement 2009-11-18 16:33:01 EST

That diagnostic is interesting - glad you tried it on the command line and got me:

org.aspectj.weaver.ResolvedType.addPointcutsResolvingConflicts(ResolvedType.java:2275)

because:
org.aspectj.weaver.ResolvedType.addPointcutsResolvingConflicts(ResolvedType.java:2273)

as shown in comment 19 just has the word 'continue;' on it :)

When patching, if you are in the IDE you must patch the weaver plugin rather than the compiler plugin.  When on the command line it is all in the tools jar so that just needs patching.  Anyway - the new NPE tells us something very interesting - that the problem isn't a pointcut with a null declaring type but a null entry in the array being used to collect pointcuts.  Before we started all this there were two possible causes of the NPE (comment 3) - now we know which it is.

Comment 22 Hermann Vosseler 2009-11-18 16:56:34 EST

comment 19 showed a diff between old/new stacktrace ... indeed confusing, sorry ;-)

Comment 23 Andrew Clement 2009-11-18 17:08:29 EST

oh yes, it was a diff, doh, stupid formatting in my browser hid that.  I'm finding other code that copes with the collection containing null elements, so I may just put the guard in.  Just wish one of my 4000 tests also showed a null in that list so I could see why it happens.

thanks for collecting the debug Hermann!

Comment 24 Thomas Hofmann 2009-11-19 11:40:50 EST

(In reply to comment #10)
> Hi Hermann,
> 
> I've actually committed some debug into HEAD:
> 
> ResolvedType pointcutDeclaringType =
>    existing.getDeclaringType().resolve(getWorld());
> if (pointcutDeclaringType == null) {
>   throw new BCException("Pointcut declaring type is unexpectedly null. 
> Pointcut is " + existing.toString());
> }
> if (!isVisible(existing.getModifiers(), pointcutDeclaringType, this)) {
> 
> if you were able to just grab AspectJ 1.6.7 (latest dev build) and run with it,
> the debug should tell us at least which pointcut is giving rise to the issue -
> that may help me recreate.  (The latest AJDT dev builds include this change
> too)

Hi,

I would also like to provide some more infos on where it fails in my case. But unfortunately, there is no AJDT with 1.6.7 for eclipse 3.4 yet :)

Comment 25 Thomas Hofmann 2009-11-19 11:44:59 EST

(In reply to comment #24)

Do you know which dev build of AJDT introduced this?

Comment 26 Andrew Clement 2009-11-19 11:53:04 EST

Hi Thomas,

per Andrews message yesterday on the AJDT list, the latest dev builds for Eclipse 3.4 and 3.5 include AspectJ 1.6.7.

the debug I eluded to earlier in this thread is insufficient to find the problem unfortunately.

Comment 27 Andrew Clement 2009-11-19 12:19:36 EST

I found a possible case that could lead to a null occurring downstream and triggering that NPE.  Not 100% sure it is the cause here though as it *looked* like it would only occur if there was an error in the incoming source code.  However, I've committed the fix for allowing the null to travel downstream plus a guard in the code encountering the null.

Comment 28 Thomas Hofmann 2009-11-19 13:42:22 EST

(In reply to comment #27)
> I found a possible case that could lead to a null occurring downstream and
> triggering that NPE.  Not 100% sure it is the cause here though as it *looked*
> like it would only occur if there was an error in the incoming source code. 
> However, I've committed the fix for allowing the null to travel downstream plus
> a guard in the code encountering the null.

Well, how / when can we try it out?

Comment 29 Andrew Clement 2009-11-19 13:51:49 EST

I'll patch AJDT shortly, that will be for Eclipse 3.5.  3.4 will follow a little after that again...

Comment 30 Andrew Clement 2009-11-27 15:33:28 EST

latest AJDT for 3.5 includes the guard changed proposed here.

Comment 31 Andrew Clement 2010-01-11 13:44:22 EST

hey guys - have you seen any changes with this bug in recent AJDTs?

Comment 32 Andrew Clement 2010-01-25 16:59:19 EST

guard is in. please reopen if still happening.