Skip to content
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

Per-classloader weaving cache fails for classes generated for around advice types #315

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kriegaex
Copy link
Contributor

Relates to #314.

by refining and extending org.aspectj.systemtest.ajc171.NewFeatures,
also considering around advice types and repeated weaving, actually
re-using the cache.

Signed-off-by: Alexander Kriegisch <[email protected]>
@kriegaex kriegaex self-assigned this Jul 18, 2024
@kriegaex
Copy link
Contributor Author

kriegaex commented Jul 18, 2024

@KimmingLau, I am busy relocating long-distance and do not have much time now. I pushed a commit extending an existing test, reproducing the problem. If you feel like taking a look, that would be terrific.

It looks as if this scenario was never tested before and hence never worked to begin with.

@KimmingLau
Copy link
Contributor

KimmingLau commented Jul 19, 2024

Congratulations on the new house! I am also very interested in this problem, and I have reproduced it. I think the root cause of this problem is that the related generated class cached bytes (Here is Foo$AjcClosure1.class) was not define in advance before returning the class cached bytes (Here is Foo.class). By contrast, shard cache has done this operation in org.aspectj.weaver.tools.cache.SimpleCache#initializeClass(), so there is no such problem when using -Daj.weaving.cache.impl=shared.

@kriegaex
Copy link
Contributor Author

kriegaex commented Jul 19, 2024

Yes, the generated inner class is undefined, which is why the outer class referencing it cannot be fully loaded. If you look at the failure stack trace, you see no AspectJ classes in the trace at all, i.e. when the woven outer class is loaded in WeavedClassCache mode, the inner class is not loaded on time, whereas in SimpleCache (shared cache) mode, this happens by whatever means, maybe via defineClass, which does not exist equivalently in WeavedClassCache. The implementation is simply incomplete, I guess. It was never tested automatically before 2f9c242, therefore nobody noticed until the GitHub issue was raised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants