search

eclipse-aspectj / aspectj

Other
272 stars 82 forks source link

`StackOverflowError` after converting an aspect from `.aj` to `.java` #310

Closed lefou closed 1 month ago

lefou commented 2 months ago

Here is the head of the error message:

Exception thrown from AspectJ 1.9.22
...
when batch building BuildConfig[null] #Files=0 AopXmls=#0
null
java.lang.StackOverflowError
    at org.aspectj.weaver.patterns.BasicTokenSource.peek(BasicTokenSource.java:51)
    at org.aspectj.weaver.patterns.PatternParser.maybeEat(PatternParser.java:1875)
    at org.aspectj.weaver.patterns.PatternParser.parseNotOrPointcut(PatternParser.java:363)
    at org.aspectj.weaver.patterns.PatternParser.parseNotOrPointcut(PatternParser.java:364)
    at org.aspectj.weaver.patterns.PatternParser.parseNotOrPointcut(PatternParser.java:364)
    at org.aspectj.weaver.patterns.PatternParser.parsePointcut(PatternParser.java:351)
    at org.aspectj.weaver.bcel.AtAjAttributes.parsePointcut(AtAjAttributes.java:1957)
    at org.aspectj.weaver.bcel.AtAjAttributes.handlePointcutAnnotation(AtAjAttributes.java:1422)
    at org.aspectj.weaver.bcel.AtAjAttributes.readAj5ClassAttributes(AtAjAttributes.java:352)
    at org.aspectj.weaver.bcel.BcelObjectType.ensureAspectJAttributesUnpacked(BcelObjectType.java:394)
    at org.aspectj.weaver.bcel.BcelObjectType.<init>(BcelObjectType.java:161)
    at org.aspectj.weaver.bcel.BcelWorld.buildBcelDelegate(BcelWorld.java:412)
    at org.aspectj.weaver.bcel.BcelWorld.resolveDelegate(BcelWorld.java:407)
    at org.aspectj.weaver.World.resolveToReferenceType(World.java:486)
    at org.aspectj.weaver.World.resolve(World.java:321)
...
Mai 02, 2024 4:31:38 PM org.aspectj.weaver.tools.Jdk14Trace info
INFORMATION: Dumping to /home/lefou/..././ajcore.20240502.163138.299.txt

In the failing sub-module accounting, we use the aspect compiler to weave-compile the classes given via the -inpath. The classes itself are built with kotlinc and javac. This setup used to work before. The only change happened in a upstream sub-module kernel, which is consumed via the -aspectpath. In that module, we changed one aspect from .aj to .java using the @Aspect 5 annotations. The kernel module itself uses the aspect compiler to compile the source code and compiles fine.

I'm also adding the ajcore.20240502.112934.013.txt dump file. The project isn't open source. The build tools used is Mill with the mill-aspectj plugin. Therefore I don't have a shareable reproducer yet. I'm reporting the error in the hope, the issue can be analyzed with the already provided information. If not, I'm happy to provide more information and/or create a reproducer. But since this isn't easy on my side, I'm waiting to an explicit request. Maybe, it's not necessary at all?

kriegaex commented 2 months ago

It is really difficult to debug invisible code. The stack trace looks as if

Please make sure I have access to a minimal reproducer. Moreover, does the problem only affect 1.9.22 or also older AspectJ versions, say 1.9.19, 1.9.7, 1.8.14? It would be helpful to limit the search space. Thank you.

kriegaex commented 2 months ago

Note to myself: The ajcore file shows an endless loop (118 occurrences before SOE) of

at org.aspectj.weaver.patterns.TypePattern.resolveExactType(TypePattern.java:217)
at org.aspectj.weaver.patterns.ReferencePointcut.resolveBindings(ReferencePointcut.java:126)
at org.aspectj.weaver.patterns.Pointcut.resolve(Pointcut.java:189)
at org.aspectj.weaver.patterns.PerCflow.resolveBindings(PerCflow.java:77)
at org.aspectj.weaver.patterns.Pointcut.resolve(Pointcut.java:189)
at org.aspectj.weaver.AjAttribute$Aspect.reifyFromAtAspectJ(AjAttribute.java:664)
at org.aspectj.weaver.bcel.BcelObjectType.ensureAspectJAttributesUnpacked(BcelObjectType.java:408)
at org.aspectj.weaver.bcel.BcelObjectType.<init>(BcelObjectType.java:161)
at org.aspectj.weaver.bcel.BcelWorld.buildBcelDelegate(BcelWorld.java:412)
at org.aspectj.weaver.bcel.BcelWorld.resolveDelegate(BcelWorld.java:407)
at org.aspectj.weaver.World.resolveToReferenceType(World.java:486)
at org.aspectj.weaver.World.resolve(World.java:321)
at org.aspectj.weaver.patterns.SimpleScope.lookupType(SimpleScope.java:92)
at org.aspectj.weaver.BindingScope.lookupType(BindingScope.java:66)
at org.aspectj.weaver.patterns.WildTypePattern.lookupTypeInScope(WildTypePattern.java:745)
at org.aspectj.weaver.patterns.WildTypePattern.resolveBindingsFromFullyQualifiedTypeName(WildTypePattern.java:733)
at org.aspectj.weaver.patterns.WildTypePattern.resolveBindings(WildTypePattern.java:662)

before topping out at what was posted in the question. I.e., the question is why this happens.

lefou commented 2 months ago

Thank you @kriegaex ! Since the stack overflow seems to be related to the aspect we changed, I'll try to minimize the issue to come up with a reproducer. Maybe, we either made some mistake when translating from aspect to class notation, or there is indeed a issue. Ideally, I just find the issue while going.

kriegaex commented 2 months ago

Maybe, we either made some mistake when translating from aspect to class notation, or there is indeed a issue. Ideally, I just find the issue while going.

Yes, that is possible. It is one of the beneficial side effects of creating an MCVE that it helps you to understand the situation better.

If you are in a position to show me the code before/after conversion privately, feel free to find some chat contact data in my Stack Overflow profile (same user name). If you are lucky, I can spot something quickly.

lefou commented 2 months ago

Nice move. I guess the aspect itself isn't that much of a secret, except being legacy as hell. 😵‍💫

This is just the migrated aspect, which is not the only in the code base, but the only change. See revision 2 of this gist: https://gist.github.com/lefou/bd625e7e1a00dfeb63546e600ebeed4e/revisions

The idea behind the migration is to get better navigation support in IntelliJ IDEA and rework the code base and finally clean up. AspectJ development was much easier with Eclipse, tbh.

kriegaex commented 2 months ago

I cannot compile this aspect, because it imports not just lots of Spring classes but also your own classes and uses a pointcut from another aspect. The aspect is a bit too complex to parse only in my head.

lefou commented 2 months ago

Understood. I'll try to prepare a reproducer. Does it need to be a Maven project, or would be Mill ok, given some instructions to reproduce?

kriegaex commented 2 months ago

I added some Spring dependencies and the imported application classes and aspects to a sample Maven project, and it compiles fine. Waiting for your reproducer.

kriegaex commented 2 months ago

Does it need to be a Maven project, or would be Mill ok

I just noticed this part of the question now. I have never used or even seen Mill before, i.e. if you would remove this barrier, it would be nice. But if the build file is comprehensible enough for me to map the dependencies and compiler options to Maven myself, I can give it a try first and complain afterwards, if necessary.

kriegaex commented 1 month ago

Any news here? I am asking, because some people wait for release 1.9.22.1, and if there is a bug that I can fix and include it, I would like it to be part of that release. Otherwise, it would have to wait at least until 1.9.23.

lefou commented 1 month ago

No relevant updates, yet. Please don't hold back your next release.

Since the issue doesn't occur in the .aj version of the aspect, we started to simplify that version instead. I wasn't able to isolate the issue to a reasonable smaller project and currently lack the time to continue that effort. Feel free to close this issue, since I can't verify that it's a bug in AspectJ. I can comment/reopen, once I find more evidence.

kriegaex commented 1 month ago

Understood. AspectJ 1.9.22.1 was released without any potential fixes for this issue. Closing as requested.