diff options
author | Alexander Kriegisch <Alexander@Kriegisch.name> | 2024-01-28 16:13:22 +0700 |
---|---|---|
committer | Alexander Kriegisch <Alexander@Kriegisch.name> | 2024-01-29 07:07:26 +0700 |
commit | f986c3d18386e77c974e4272ab27c3b573773c9b (patch) | |
tree | 542e0be08b7f34133dc9813cef864e5bfb75e31d /testing | |
parent | 8b3a50e5ab8d78e96dd4401081388bc34c17edf4 (diff) | |
download | aspectj-f986c3d18386e77c974e4272ab27c3b573773c9b.tar.gz aspectj-f986c3d18386e77c974e4272ab27c3b573773c9b.zip |
Workaround for defining classes during LTW
Overhaul ClassLoaderWeavingAdaptor to use statically initialised Unsafe
instances and method handles pointing to their 'defineClass' methods.
Those now work universally on JDKs 8-21. In older JDKs, the method used
to be in sun.misc.Unsafe, in more recent ones on jdk.internal.misc.Unsafe.
It is challenging to fetch instances, especially as reflection
protection and module boundaries have been increased in the JDK
progressively. But finally, a solution was adapted from Byte Buddy (BB).
Kudos to BB author Rafael Winterhalter. The previous solution to use
ClassLoader::defineClass and require '--add-opens' is no longer
necessary for the first time since it became necessary in AspectJ 1.9.7
with Java 16 support.
Add org.ow2.asm:asm-common as a dependency everywhere org.ow2.asm:asm
was used before. Maybe that is too many places, but no worse than before.
Add missing dependency on loadtime to aspectjweaver. This kept a build
like "mvn install -am -pl aspectjweaver" from picking up changed
loadtime classes.
Fixes #117.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Diffstat (limited to 'testing')
-rw-r--r-- | testing/pom.xml | 4 | ||||
-rw-r--r-- | testing/src/test/java/org/aspectj/testing/AntSpec.java | 12 | ||||
-rw-r--r-- | testing/src/test/java/org/aspectj/testing/RunSpec.java | 15 |
3 files changed, 24 insertions, 7 deletions
diff --git a/testing/pom.xml b/testing/pom.xml index 38784790a..c5e082255 100644 --- a/testing/pom.xml +++ b/testing/pom.xml @@ -53,6 +53,10 @@ <artifactId>asm</artifactId> </dependency> <dependency> + <groupId>org.ow2.asm</groupId> + <artifactId>asm-commons</artifactId> + </dependency> + <dependency> <groupId>org.aspectj</groupId> <artifactId>testing-client</artifactId> <version>${project.version}</version> diff --git a/testing/src/test/java/org/aspectj/testing/AntSpec.java b/testing/src/test/java/org/aspectj/testing/AntSpec.java index 7a6bd3cd9..33668d509 100644 --- a/testing/src/test/java/org/aspectj/testing/AntSpec.java +++ b/testing/src/test/java/org/aspectj/testing/AntSpec.java @@ -98,9 +98,15 @@ public class AntSpec implements ITestStep { // setup aj.dir "modules" folder p.setUserProperty("aj.root", new File("..").getAbsolutePath()); - // On Java 16+, LTW no longer works without this parameter. Add the argument here and not in AjcTestCase::run, - // because even if 'useLTW' and 'useFullLTW' are not set, we might in the future have tests for weaver attachment - // during runtime. See also docs/release/README-1.8.7.html. + // On Java 16+, LTW did not work on AspectJ 1.9.7 to 1.9.21 without this parameter. So, we added the argument here + // and not in AjcTestCase::run, because without 'useLTW' or 'useFullLTW', there might have been a need for weaver + // attachment during runtime. See also docs/release/README-1.8.7.adoc. + // + // Since AspectJ 1.9.21.1, '--add-opens' is no longer necessary, because we found a workaround for defining + // classes in arbitrary class loaders. But some tests, e.g. AtAjLTWTests.testLTWUnweavable, still use + // ClassLoader::defineClass to inject dynamically generated classes into the current class loader. Therefore, we + // still set the parameters, so they can be used on demand - not for LTW as such, but for class injection. See + // also tests/java5/ataspectj/ataspectj/UnweavableTest.java. // // Attention: Ant 1.6.3 under Linux neither likes "" (empty string) nor " " (space), on Windows it would not be // a problem. So we use "_dummy" Java system properties, even though they pollute the command line. diff --git a/testing/src/test/java/org/aspectj/testing/RunSpec.java b/testing/src/test/java/org/aspectj/testing/RunSpec.java index 593c5b957..4d7526f44 100644 --- a/testing/src/test/java/org/aspectj/testing/RunSpec.java +++ b/testing/src/test/java/org/aspectj/testing/RunSpec.java @@ -68,14 +68,21 @@ public class RunSpec implements ITestStep { if (vmargs == null) vmargs = ""; - // On Java 16+, LTW no longer works without this parameter. Add the argument here and not in AjcTestCase::run, - // because even if 'useLTW' and 'useFullLTW' are not set, we might in the future have tests for weaver attachment - // during runtime. See also docs/release/README-1.8.7.html. + // On Java 16+, LTW did not work on AspectJ 1.9.7 to 1.9.21 without this parameter. So, we added the argument here + // and not in AjcTestCase::run, because without 'useLTW' or 'useFullLTW', there might have been a need for weaver + // attachment during runtime. See also docs/release/README-1.8.7.adoc. + // + // Since AspectJ 1.9.21.1, '--add-opens' is no longer necessary, because we found a workaround for defining + // classes in arbitrary class loaders. But some tests, e.g. AtAjLTWTests.testLTWUnweavable, still use + // ClassLoader::defineClass to inject dynamically generated classes into the current class loader. Therefore, we + // still set the parameters, so they can be used on demand - not for LTW as such, but for class injection. See + // also tests/java5/ataspectj/ataspectj/UnweavableTest.java. // // The reason for setting this parameter for Java 9+ instead of 16+ is that it helps to avoid the JVM printing // unwanted illegal access warnings during weaving in 'useFullLTW' mode, either making existing tests fail or // having to assert on the warning messages. - vmargs += is16VMOrGreater() ? " --add-opens java.base/java.lang=ALL-UNNAMED" : ""; + // + // vmargs += is16VMOrGreater() ? " --add-opens java.base/java.lang=ALL-UNNAMED" : ""; AjcTestCase.RunResult rr = inTestCase.run(getClassToRun(), getModuleToRun(), args, vmargs, getClasspath(), getModulepath(), useLtw, "true".equalsIgnoreCase(usefullltw)); |