-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue with equinox weaving on Eclipse version 2024-03 #584
Comments
To the Equinox team: This is the AspectJ maintainer. I have zero knowledge about Equinox Weaving, but if you have any questions about AspectJ and the changes I mentioned in eclipse-aspectj/ajdt#57, by all means feel free to mention my user ID and ask me questions. |
@martinlippert Do you know what could cause this? |
@kriegaex I'm not sure what exactly is considered "changed" here something in AspectJ? Some change (wich one?) in Equinox or JDT? |
Equinox weaving predates OSGi Weaving Hook Service Specification. The AspectJ weaving hooks into Equinox Weaving. Equinox Weaving is built upon Equinox Framework specific behavior. The AspectJ weaving use to live with the Equinox project but since got moved out of the project. I don't recall now where they went but I think this issue belongs with that project. @martinlippert is the last developer I know that has touched the AspectJ weaving integration with Equinox Weaving. |
@laeubi, I am reiterating: I have zero knowledge about Equinox Weaving. I do not even know if it is the root cause of the problem at all. Therefore, I am not claiming that anything is wrong there. I just made an educated guess, as LTW with plain AspectJ seems to work fine, both inside and outside of Eclipse, and I have yet to see a reproducer for the problem with a simple AspectJ LTW configuration in Eclipse. What has changed in AspectJ that I think might have an impact on what is woven or not is in these commits:
The main affected weaver classes are:
Like I said in the comment I linked to earlier:
The rationale behind the change is that now the AspectJ weaver behaves like most intrumentation agents, simply returning |
Also see https://bugs.eclipse.org/bugs/show_bug.cgi?id=470000 where the aspectj weaving integration moved out of Equinox. I don't believe there is anything for Equinox to do here. |
@tjwatson, sorry for the stupid question, but where did the AspectJ integration move to, except for the parts located in AJDT? |
Not a stupid question. It is an answer I was searching for, but have not found (besides the discussions in https://bugs.eclipse.org/bugs/show_bug.cgi?id=470000). I am hoping @martinlippert sees this and can chime in. |
https://bugs.eclipse.org/bugs/show_bug.cgi?id=470000#c33 The overall, AspectJ independent parts of the weaving infrastructure that we've built back then remained in Equinox, whereas the AspectJ specific implementation of the weaving moved out to the AJDT project back then. I haven't looked at this code for years now, so can't really say anything from the top of my head. Looks like some debugging would be needed here in order to find out whether the weaver creates the additional (missing) bytecode and if so, check if the weaving infrastructure in Equinox correctly retrieves and uses this. Since I am not actively working on this anymore, I wonder whether we should deprecate this implementation and load-time weaving of AspectJ aspects in Equinox for future releases. Opinions @tjwatson ? |
The part that uses the home-grown weaving infrastructure in Equinox to do AspectJ weaving is here: https://github.com/eclipse-aspectj/ajdt/tree/master/org.eclipse.equinox.weaving.aspectj |
The main implementation of the AspectJ lead-time weaving is implemented in here: https://github.com/eclipse-aspectj/ajdt/blob/master/org.eclipse.equinox.weaving.aspectj/src/org/eclipse/equinox/weaving/aspectj/loadtime/OSGiWeavingAdaptor.java - it calls and re-uses the |
Right, I do think we need to deprecate Equinox weaving and ultimately stop building it in our quarterly releases. That is unless we get someone to step up that has time and interest to maintain these integrations. |
@martinlippert, thanks for the hint about |
From a very high level, it looks like the weaver should have produced bytecode for this new inner class There is special handling for generated classes in the mechanism, you will stumble upon it when looking at the code. The basic idea is to keep the generated bytecodes for additional classes in memory until the JVM loads that class (as far as I remember). |
I also vaguely remember a PR being raised a while ago, fixing problems with race conditions in this area. You might want to reach out to the reporter here #234, who did a great and very detailed job on this - just in case. |
I think, I was able to locate the code in AJDT responsible for the problem. It is about defining classes in a different classloader, which both AspectJ and AJDT used the same method for. In AspectJ, this was updated to work with more recent JDKs already, but in AJDT there was still a copy of the old code. It is not so much a bug as a missing, but necessary adjustment to new Java runtimes. Thanks @martinlippert and @tjwatson for providing the context information necessary for me to zero in on the problem. I think, the issue can be closed. I am going to fix the original problem in eclipse-aspectj/ajdt#57. |
As per previous comment. |
Sounds good, thanks a lot @kriegaex for looking into this, much appreciated. |
@martinlippert, maybe we were too optimistic to close the issue. The error still occurs, just in another situation. Would you mind reading the thread from eclipse-aspectj/ajdt#57 (comment) downwards? Sorry, quite a lot of text. Maybe you have an idea what has changed in Equinox weaving that could cause this or what AJDT needs to accomodate to to avoid the problem. Thank you. |
@kriegaex replied on the AspectJ issue via eclipse-aspectj/ajdt#57 (comment) |
I have already had some discussions on another GitHub project eclipse-aspectj/ajdt#57
In short: I have an Eclipse product consisting of some official plugins and my own plugin which uses AspectJ. After building it with Maven/Tycho, it works correctly on Linux but not on the Windows platform. This issue seems to have arisen since version 2023-12 of Eclipse. I have been using the same code for years now, and for example, on version 2023-06 of Eclipse, it was working correctly.
I have managed to prepare a small example of such a product: example.zip
It can be built with Maven 3.9+ using the
mvn package
command.The reproduction scenario is simply to launch the application, and an exception is thrown as shown below:
I am not sure if this is a bug or invalid configuration but any help would be appreciated.
The text was updated successfully, but these errors were encountered: