AJC throws NPE when number of classpath entries is over the limit

[raimondas67]

1. Java 11 (AdoptOpenJDK jdk-11.0.10.9-hotspot)
2. aspectj-maven-plugin v1.14
3. aspectjtools v1.9.7
4. pom.xml snippet

org.codehaus.mojo aspectj-maven-plugin 1.14 11 11 11 true true xxx.xxx.xxx xxx-aspects **/*.aj **/*.class cantFindType=ignore ${project.build.directory}/classesToWeav process-classes compile org.aspectj aspectjtools 1.9.7 compile

5. Stack trace:
   [ERROR] Failed to execute goal org.codehaus.mojo:aspectj-maven-plugin:1.14.0:compile (default) on project xxx: AJC compiler errors:
   [ERROR] abort ABORT -- (NullPointerException) null
   [ERROR] null
   [ERROR] java.lang.NullPointerException
   [ERROR] at org.aspectj.org.eclipse.jdt.internal.compiler.batch.ClasspathJmod.getModulesDeclaringPackage(ClasspathJmod.java:146)
   [ERROR] at org.aspectj.org.eclipse.jdt.internal.compiler.batch.ClasspathLocation.isPackage(ClasspathLocation.java:184)
   [ERROR] at org.aspectj.org.eclipse.jdt.internal.compiler.batch.ClasspathJmod.findClass(ClasspathJmod.java:55)
   [ERROR] at org.aspectj.org.eclipse.jdt.internal.compiler.batch.FileSystem.internalFindClass(FileSystem.java:544)
   [ERROR] at org.aspectj.org.eclipse.jdt.internal.compiler.batch.FileSystem.findClass(FileSystem.java:464)
   [ERROR] at org.aspectj.org.eclipse.jdt.internal.compiler.batch.FileSystem.findType(FileSystem.java:617)
   [ERROR] at org.aspectj.org.eclipse.jdt.internal.compiler.env.IModuleAwareNameEnvironment.findType(IModuleAwareNameEnvironment.java:101)
   [ERROR] at org.aspectj.ajdt.internal.core.builder.StatefulNameEnvironment.findType(StatefulNameEnvironment.java:101)
   [ERROR] at org.aspectj.org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.createPlainPackage(LookupEnvironment.java:1151)
   [ERROR] at org.aspectj.org.eclipse.jdt.internal.compiler.lookup.CompilationUnitScope.buildTypeBindings(CompilationUnitScope.java:136)
   [ERROR] at org.aspectj.org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.buildTypeBindings(LookupEnvironment.java:487)
   [ERROR] at org.aspectj.ajdt.internal.compiler.lookup.AjLookupEnvironment.buildTypeBindings(AjLookupEnvironment.java:1471)
   [ERROR] at org.aspectj.org.eclipse.jdt.internal.compiler.Compiler.internalBeginToCompile(Compiler.java:870)
   [ERROR] at org.aspectj.org.eclipse.jdt.internal.compiler.Compiler.beginToCompile(Compiler.java:395)
   [ERROR] at org.aspectj.org.eclipse.jdt.internal.compiler.Compiler.compile(Compiler.java:449)
   [ERROR] at org.aspectj.org.eclipse.jdt.internal.compiler.Compiler.compile(Compiler.java:427)
   [ERROR] at org.aspectj.ajdt.internal.core.builder.AjBuildManager.performCompilation(AjBuildManager.java:1096)
   [ERROR] at org.aspectj.ajdt.internal.core.builder.AjBuildManager.performBuild(AjBuildManager.java:275)
   [ERROR] at org.aspectj.ajdt.internal.core.builder.AjBuildManager.batchBuild(AjBuildManager.java:188)
   [ERROR] at org.aspectj.ajdt.ajc.AjdtCommand.doCommand(AjdtCommand.java:103)
   [ERROR] at org.aspectj.ajdt.ajc.AjdtCommand.runCommand(AjdtCommand.java:47)
   [ERROR] at org.aspectj.tools.ajc.Main.run(Main.java:372)
   [ERROR] at org.aspectj.tools.ajc.Main.runMain(Main.java:250)
   [ERROR] at org.codehaus.mojo.aspectj.AbstractAjcCompiler.execute(AbstractAjcCompiler.java:568)
   [ERROR] at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:137)
   [ERROR] at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:210)
   [ERROR] at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:156)
   [ERROR] at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:148)
   [ERROR] at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:117)
   [ERROR] at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:81)
   [ERROR] at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:56)
   [ERROR] at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:128)
   [ERROR] at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:305)
   [ERROR] at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:192)
   [ERROR] at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:105)
   [ERROR] at org.apache.maven.cli.MavenCli.execute(MavenCli.java:956)
   [ERROR] at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:288)
   [ERROR] at org.apache.maven.cli.MavenCli.main(MavenCli.java:192)
   [ERROR] at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   [ERROR] at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   [ERROR] at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   [ERROR] at java.base/java.lang.reflect.Method.invoke(Method.java:566)
   [ERROR] at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:289)
   [ERROR] at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
   [ERROR] at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
   [ERROR] at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)
   [ERROR]

The problem is that ClasspathJmod objects for JMOD files have zipFile=null

Problem can be solved by providing system property -Dorg.aspectj.weaver.openarchives=2000 when running maven build. It turns out due to big number of classpath entries some of these entries are closed during processing:

org.aspectj.org.eclipse.jdt.internal.compiler.batch.ClasspathJar

// AspectJ Extension
private void ensureOpen() throws IOException { //line 403
if (zipFile != null) return; // If its not null, the zip is already open
if (openArchives.size()>=maxOpenArchives) {
closeSomeArchives(openArchives.size()/10); // Close 10% of those open <-- closed and likely not re-initialized so that zipFile remains null
}
zipFile = new ZipFile(file);
openArchives.add(this);
}

[aclement]

The fix here seems relatively straightforward I suspect. The Java Module support added in Java9 probably added new routes that are attempting to access the zipFile object. We should look at those routes and check if they are guarded by a call to ensureOpen() - if they are not then it should be added. Basically all attempts to access that zipFile field need to be guarded in case it has been closed due to too many archives being simultaneously open.

[kriegaex]

I have done quick local fix, adding ensureOpen() to several routes where theoretically there could be access to a closed jar/zip file. This fixes the problem for me in the test scenario I described on the mailing list, i.e. compilation of a simple class with -Dorg.aspectj.weaver.openarchives=20, which would otherwise always fail of the (too) small number of open archives. I am going to analyse if maybe I can remove snippet from a few routes where it is done multiple times, but I think better safe than sorry. I also had to make ensureOpen() protected to be able to call it from a subclass where it was necessary. I simply added this to most places, ...

// AspectJ Extension
try {
  ensureOpen();
}
catch (IOException e) {
  throw new RuntimeException(e);
}
// End AspectJ Extension

... rather than catching the error and logging it or returning null or false. I think we want compilation to fail if an error happens in that method, don't we, @aclement? In some places I saw that we return values instead or just fold the ensureOpen() call into an existing try-catch block. WDYT?

---

Update: Andy, shouldn't this be a set rather than a list?

private static List openArchives = new ArrayList();

There seems to be the danger of duplicated entries there, especially if ensureOpen() is called from many places and looks like this?

private void ensureOpen() throws IOException {
	if (zipFile != null) return; // If its not null, the zip is already open
	if (openArchives.size()>=maxOpenArchives) {
		closeSomeArchives(openArchives.size()/10); // Close 10% of those open
	}
	zipFile = new ZipFile(file);
	openArchives.add(this);
}

If zipFile != null, it will be added to the list unconditionally, which looks like it will not just bloat the list, but also make it inconsistent.

[kriegaex]

@raimondas67, I deployed an AspectJ 1.9.9-SNAPSHOT to Sonatype OSSRH. Please test if your problem goes away without -Dorg.aspectj.weaver.openarchives=2000 and maybe even try to stress-test the situation more by setting a lower limit. I know, it is probably going to make compilation slower, but try to experiment first without the setting (defaults to 1000), then with 500, 250, 100 or even 50, if it still continues to work like that. I just want to know if the error is gone.

See #95 (comment) for an example project showing how to upgrade to the latest AspectJ version when using dev.aspectj:aspectj-maven-plugin:1.13.1. Just make sure to set the aspectjtools snapshot as a plugin dependency.

As for your second problem from the mailing list, I think you still have not created an issue for it. Can you help us to reproduce it?

[kriegaex]

@aclement, I inspected the existing logic and think it is safe to continue using the list instead of a set, mainly because it is more predictable and chronological which classpath JARs get closed on cache exhaustion (first in, first out). We would have to use a sorted set or map with a sortable timestamp or so as the key, which would entail more changes than I want to implement now. But I think I was pretty thorough with where to put the safeguards, my checks also encompassing subclasses. I think that for now we are OK, at least no worse than before. If @raimondas67 also re-tests successfully, I think the two PRs for JDT Core and AspectJ are ready to be reviewed and merged, whenever you have cycles to look into them.

[kriegaex]

For the record: In addition to the regression test I added, which always runs with -Dorg.aspectj.weaver.openarchives=20, I also pushed a private branch with commit eab7758, resulting in the full build with hundreds of tests doing AJC compilation also with -Dorg.aspectj.weaver.openarchives=20. The build passed. Because old builds will eventually be deleted on GitHub, here is a screenshot:

[raimondas67]

Hi, Classpath related NPE exception is fixed with this 1.9.9-SNAPSHOT release, however, I’m still getting the same AJC compiler error [ERROR] Failed to execute goal org.codehaus.mojo:aspectj-maven-plugin:1.14.0:compile (default) on project xxx-app: AJC compiler errors: [ERROR] error at (no source information available) [ERROR] C:\Users\....aj:0::0 Type java.lang.Object is indirectly referenced from required .class files but cannot be resolved since the declaring package java.lang exported from module java.base conflicts with a package accessible from module <unnamed> thus 1.9.4 has to be used. Regards From: Alexander Kriegisch Sent: Wednesday, February 23, 2022 5:04 PM To: eclipse/org.aspectj Cc: raimondas67 ; Mention Subject: Re: [eclipse/org.aspectj] AJC throws NPE when number of classpath entries is over the limit (Issue #125) @aclement, I inspected the existing logic and think it is safe to continue using the list instead of a set, mainly because it is more predictable and chronological which classpath JARs get closed on cache exhaustion (first in, first out). We would have to use a sorted set or map with a sortable timestamp or so as the key, which would entail more changes than I want to implement now. But I think I was pretty thorough with where to put the safeguards, my checks also encompassing subclasses. I think that for now we are OK, at least no worse than before. If @raimondas67 also re-tests successfully, I think the two PRs for JDT Core and AspectJ are ready to be reviewed and merged, whenever you have cycles to look into them. — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.Message ID: ***@***.***>

[kriegaex]

@raimondas67, please try not to use full quotes. You could also edit the issue description, using code blocks for better formatting.

Classpath related NPE exception is fixed with this 1.9.9-SNAPSHOT release

Thanks for the feedback, good news. 🙂

however, I’m still getting the same AJC compiler error

Yes, of course. I did not fix it, having too little information to reproduce it. I never thought that the two issues were related anyway, which is why I already asked you twice to create an extra issue with more information for the second issue. Thanks in advance.