-
Notifications
You must be signed in to change notification settings - Fork 333
Add Java 26 support to CI Visibility #10839
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
Changes from 9 commits
2d8960b
fd887e0
ce0dcf3
d3ad1b1
34053f5
25dca90
1869284
c30dfe5
082abbf
adb3df5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| package datadog.trace.civisibility.compiler; | ||
|
|
||
| import datadog.trace.util.JDK9ModuleAccess; | ||
| import java.lang.instrument.ClassFileTransformer; | ||
| import java.lang.instrument.Instrumentation; | ||
| import java.security.ProtectionDomain; | ||
| import java.util.Collections; | ||
| import java.util.Set; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| /** | ||
| * Exports jdk.compiler internal packages to the classloader that loads dd-javac-plugin. | ||
| * | ||
| * <p>On JDK 16+ (strong encapsulation), dd-javac-plugin's CompilerModuleOpener uses burningwave to | ||
| * export these packages. On JDK 26+, burningwave fails due to Unsafe restrictions (JEP 471/498). | ||
| * This transformer intercepts dd-javac-plugin class loading and does the export using | ||
| * Instrumentation.redefineModule() instead. | ||
| * | ||
| * <p>Each Maven compilation step (compile, testCompile) may use a different classloader, so we | ||
| * track which classloaders have already been exported to and re-export for new ones. | ||
| */ | ||
| public class CompilerModuleExporter implements ClassFileTransformer { | ||
|
|
||
| private static final Logger LOGGER = LoggerFactory.getLogger(CompilerModuleExporter.class); | ||
|
|
||
| private static final String COMPILER_PLUGIN_CLASS_PREFIX = "datadog/compiler/"; | ||
| private static final String[] COMPILER_PACKAGES = { | ||
| "com.sun.tools.javac.api", | ||
| "com.sun.tools.javac.code", | ||
| "com.sun.tools.javac.comp", | ||
| "com.sun.tools.javac.tree", | ||
| "com.sun.tools.javac.util" | ||
| }; | ||
|
|
||
| private final Instrumentation inst; | ||
| private final Set<ClassLoader> exportedClassLoaders = | ||
| Collections.newSetFromMap(new ConcurrentHashMap<>()); | ||
|
|
||
| public CompilerModuleExporter(Instrumentation inst) { | ||
| this.inst = inst; | ||
| } | ||
|
|
||
| @Override | ||
| public byte[] transform( | ||
| ClassLoader loader, | ||
| String className, | ||
| Class<?> classBeingRedefined, | ||
| ProtectionDomain protectionDomain, | ||
| byte[] classfileBuffer) { | ||
| if (loader != null | ||
| && className != null | ||
| && className.startsWith(COMPILER_PLUGIN_CLASS_PREFIX) | ||
| && exportedClassLoaders.add(loader)) { | ||
| try { | ||
| JDK9ModuleAccess.exportModuleToUnnamedModule( | ||
| inst, "jdk.compiler", COMPILER_PACKAGES, loader); | ||
| LOGGER.debug("Exported jdk.compiler to classloader {}", loader); | ||
| } catch (Throwable e) { | ||
| LOGGER.debug("Could not export jdk.compiler packages for compiler plugin", e); | ||
| } | ||
| } | ||
| return null; // no bytecode modification | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,9 @@ | |
|
|
||
| import java.lang.instrument.Instrumentation; | ||
| import java.lang.reflect.AnnotatedElement; | ||
| import java.util.Collections; | ||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
|
|
||
| /** Use standard API to work with JPMS modules on Java9+. */ | ||
|
|
@@ -33,4 +36,28 @@ public static void addModuleReads( | |
| emptySet(), | ||
| emptyMap()); | ||
| } | ||
|
|
||
| /** Exports specific packages of a named module to a classloader's unnamed module. */ | ||
| @SuppressWarnings({"rawtypes", "unchecked"}) | ||
| public static void exportModuleToUnnamedModule( | ||
| Instrumentation inst, | ||
| String moduleName, | ||
| String[] packageNames, | ||
| ClassLoader targetClassLoader) { | ||
| java.util.Optional<java.lang.Module> optModule = | ||
| java.lang.ModuleLayer.boot().findModule(moduleName); | ||
| if (!optModule.isPresent()) { | ||
| return; | ||
| } | ||
| java.lang.Module module = optModule.get(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I followed the approach that was already in use in the file out of caution, although adding the imports doesn't seem to cause any failures/side effects. @mcculls might have better context here as he did the original implementation for the class |
||
| java.lang.Module unnamedModule = targetClassLoader.getUnnamedModule(); | ||
|
|
||
| Set<java.lang.Module> target = Collections.singleton(unnamedModule); | ||
| Map<String, Set<java.lang.Module>> extraExports = new HashMap<>(); | ||
| for (String packageName : packageNames) { | ||
| extraExports.put(packageName, target); | ||
| } | ||
|
|
||
| inst.redefineModule(module, emptySet(), (Map) extraExports, emptyMap(), emptySet(), emptyMap()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, missed it during cleanup, addressed in adb3df5 |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could probably use something like
exportedClassLoaders.computeIfAbsent(loader, this::exportJdkCompilerModule)to make sure this is truly thread safe (otherwise a thread that lost the race onaddjust exits, without waiting for the module to be exported in the other threadThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense! Updated the approach in adb3df5