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

Multiple load-time-weaver elements cause repeated addition of AspectJ transformer [SPR-14373] #18946

Closed
spring-projects-issues opened this issue Jun 16, 2016 · 1 comment

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Jun 16, 2016

Tim Gokcen opened SPR-14373 and commented

If context:load-time-weaver/ or @EnableLoadTimeWeaving is specified multiple times, then multiple LoadTimeWeavers will be added to the java.lang.instrumentation transformer chain. However, because load-time-weaved classes are not reweavable, only the first LoadTimeWeaver implementation will actually work, and the others will fail, emitting errors at startup like the following:

[TomcatInstrumentableClassLoader@67073331] error at com/company/project/MyAspect.java::0 class {0} is already woven and has not been built in reweavable mode [Xlint:nonReweavableTypeEncountered]

The obvious solution is "don't specify load-time-weaver repeatedly", but this may be confusing or difficult for large projects.

For example, in a Spring project where the context is composed of multiple submodules with beans that are imported by using <import resource="..."/> elements, it makes logical sense for the submodule <beans> elements to specify context:load-time-weaver/ because they contain Aspect beans and were designed for load-time weaving. Placing context:load-time-weaver/ statement in the "top-level" context file may not make sense because it may not even be aware that some of its submodules use Aspect-Oriented-Programming. Also, multiple top-level context files may exist (e.g., for testing or for different actual context features using shared code) and this would mean putting the load-time-weaver element in all of them.

These error messages are not necessarily harmless. If context:load-time-weaver/ is specified with conflicting options, i.e., the use of the Spring AOP processor vs the full AspectJ weaver, then unexpected behaviour could result. A submodule specifying load-time-weaving without turning on AspectJ and without having an aop.xml file, but loaded near the start of the "master" context file, could prevent submodules that might require the full AspectJ weaver from working correctly merely because they were loaded later in the "master" context file.

It seems like things would be a lot easier for users if load-time-weaver instructions could be collapsed into one before java.lang.instrument.Instrumentation.addTransformer() is called. When there are conflicting options specified, the whole context should be aborted.


Affects: 4.2.6

Issue Links:

  • #15087 Add exposeProxy to @EnableAspectJAutoProxy

Referenced from: commits 981d449

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jun 22, 2016

Juergen Hoeller commented

I'm afraid this is not designed for fully merging such definitions at this point. However, the "loadTimeWeaver" beans should at least replace each other within the same application context, with the last definition winning: This seems to work fine for @EnableAspectJWeaving but not for <context:load-time-weaver with AspectJ weaving turned on, since the latter registers an AspectJWeavingEnabler bean with a freshly generated name in that case. I'll switch this to using a well-defined bean name there, like we do for the weaver itself as well as for the bean configurer aspect, providing consistent overriding of previous definitions for a start.

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

Successfully merging a pull request may close this issue.

None yet
2 participants