diff --git a/src/main/java/net/openhft/compiler/CachedCompiler.java b/src/main/java/net/openhft/compiler/CachedCompiler.java index 4f1c989..8abf86a 100644 --- a/src/main/java/net/openhft/compiler/CachedCompiler.java +++ b/src/main/java/net/openhft/compiler/CachedCompiler.java @@ -172,6 +172,13 @@ Map compileFromJava(@NotNull String className, @NotNull String javaCode, final @NotNull PrintWriter writer, MyJavaFileManager fileManager) { + return compileFromJavaResult(className, javaCode, writer, fileManager).classes; + } + + private CompilationResult compileFromJavaResult(@NotNull String className, + @NotNull String javaCode, + final @NotNull PrintWriter writer, + MyJavaFileManager fileManager) { validateClassName(className); Iterable compilationUnits; if (sourceDir != null) { @@ -186,12 +193,15 @@ Map compileFromJava(@NotNull String className, javaFileObjects.put(className, new JavaSourceFromString(className, javaCode)); compilationUnits = new ArrayList<>(javaFileObjects.values()); // To prevent CME from compiler code } + StringBuffer diagnostics = new StringBuffer(); // reuse the same file manager to allow caching of jar files boolean ok = s_compiler.getTask(writer, fileManager, new DiagnosticListener() { @Override public void report(Diagnostic diagnostic) { if (diagnostic.getKind() == Diagnostic.Kind.ERROR) { - writer.println(diagnostic); + String message = diagnostic.toString(); + writer.println(message); + diagnostics.append(message).append(System.lineSeparator()); } } }, options, null, compilationUnits).call(); @@ -202,11 +212,11 @@ public void report(Diagnostic diagnostic) { javaFileObjects.remove(className); // nothing to return due to compiler error - return Collections.emptyMap(); + return new CompilationResult(false, Collections.emptyMap(), diagnostics.toString()); } else { Map result = fileManager.getAllBuffers(); - return result; + return new CompilationResult(true, result, diagnostics.toString()); } } @@ -226,26 +236,49 @@ public Class loadFromJava(@NotNull ClassLoader classLoader, @NotNull String className, @NotNull String javaCode, @Nullable PrintWriter writer) throws ClassNotFoundException { - Class clazz = null; - Map> loadedClasses; - synchronized (loadedClassesMap) { - loadedClasses = loadedClassesMap.get(classLoader); - if (loadedClasses == null) - loadedClassesMap.put(classLoader, loadedClasses = new LinkedHashMap<>()); - else - clazz = loadedClasses.get(className); - } + Map> loadedClasses = getOrCreateLoadedClasses(classLoader); + Class clazz = getLoadedClass(loadedClasses, className); PrintWriter printWriter = writer == null ? DEFAULT_WRITER : writer; if (clazz != null) return clazz; + MyJavaFileManager fileManager = getOrCreateFileManager(classLoader); + final Map compiled = compileFromJavaOrThrow(className, javaCode, printWriter, fileManager); + + defineCompiledClasses(classLoader, loadedClasses, compiled); + return getLoadedClassOrThrow(loadedClasses, className, compiled.keySet()); + } + + private Map> getOrCreateLoadedClasses(@NotNull ClassLoader classLoader) { + synchronized (loadedClassesMap) { + Map> loadedClasses = loadedClassesMap.get(classLoader); + if (loadedClasses == null) { + loadedClasses = new LinkedHashMap<>(); + loadedClassesMap.put(classLoader, loadedClasses); + } + return loadedClasses; + } + } + + private Class getLoadedClass(Map> loadedClasses, String className) { + synchronized (loadedClassesMap) { + return loadedClasses.get(className); + } + } + + private MyJavaFileManager getOrCreateFileManager(@NotNull ClassLoader classLoader) { MyJavaFileManager fileManager = fileManagerMap.get(classLoader); if (fileManager == null) { StandardJavaFileManager standardJavaFileManager = s_compiler.getStandardFileManager(null, null, null); fileManager = getFileManager(standardJavaFileManager); fileManagerMap.put(classLoader, fileManager); } - final Map compiled = compileFromJava(className, javaCode, printWriter, fileManager); + return fileManager; + } + + private void defineCompiledClasses(@NotNull ClassLoader classLoader, + Map> loadedClasses, + Map compiled) { for (Map.Entry entry : compiled.entrySet()) { String className2 = entry.getKey(); validateClassName(className2); @@ -254,13 +287,7 @@ public Class loadFromJava(@NotNull ClassLoader classLoader, continue; } byte[] bytes = entry.getValue(); - if (classDir != null) { - String filename = className2.replaceAll("\\.", '\\' + File.separator) + ".class"; - boolean changed = writeBytes(safeResolve(classDir, filename), bytes); - if (changed) { - LOG.info("Updated {} in {}", className2, classDir); - } - } + writeClassFileIfConfigured(className2, bytes); synchronized (className2.intern()) { // To prevent duplicate class definition error synchronized (loadedClassesMap) { @@ -274,12 +301,45 @@ public Class loadFromJava(@NotNull ClassLoader classLoader, } } } - synchronized (loadedClassesMap) { - loadedClasses.put(className, clazz = classLoader.loadClass(className)); + } + + private void writeClassFileIfConfigured(String className, byte[] bytes) { + if (classDir != null) { + String filename = className.replaceAll("\\.", '\\' + File.separator) + ".class"; + boolean changed = writeBytes(safeResolve(classDir, filename), bytes); + if (changed) { + LOG.info("Updated {} in {}", className, classDir); + } + } + } + + private Class getLoadedClassOrThrow(Map> loadedClasses, + String className, + Set compiledClassNames) throws ClassNotFoundException { + Class clazz = getLoadedClass(loadedClasses, className); + if (clazz == null) { + throw new ClassNotFoundException("Compiled class " + className + + " was not defined. Compiled classes: " + compiledClassNames); } return clazz; } + private Map compileFromJavaOrThrow(@NotNull String className, + @NotNull String javaCode, + @NotNull PrintWriter printWriter, + @NotNull MyJavaFileManager fileManager) throws ClassNotFoundException { + CompilationResult compilation = compileFromJavaResult(className, javaCode, printWriter, fileManager); + if (!compilation.success) { + throw compilationFailedException(className, compilation.diagnostics); + } + + Map compiled = compilation.classes; + if (!compiled.containsKey(className)) { + throw missingCompiledClassException(className, compiled.keySet(), compilation.diagnostics); + } + return compiled; + } + /** * Update the file manager for a specific class loader. This is mainly a * testing utility and is ignored when no manager exists for the loader. @@ -327,6 +387,27 @@ static File safeResolve(File root, String relativePath) { return candidate.toFile(); } + private static ClassNotFoundException compilationFailedException(String className, String diagnostics) { + String diagnosticText = diagnostics.trim(); + String message = "Compilation failed for " + className; + if (!diagnosticText.isEmpty()) { + message += System.lineSeparator() + diagnosticText; + } + return new ClassNotFoundException(message, new IllegalStateException(message)); + } + + private static ClassNotFoundException missingCompiledClassException(String className, + Set compiledClassNames, + String diagnostics) { + String diagnosticText = diagnostics.trim(); + String message = "Compilation did not produce requested class " + className + + ". Compiled classes: " + compiledClassNames; + if (!diagnosticText.isEmpty()) { + message += System.lineSeparator() + diagnosticText; + } + return new ClassNotFoundException(message, new IllegalStateException(message)); + } + private static PrintWriter createDefaultWriter() { OutputStreamWriter writer = new OutputStreamWriter(System.err, StandardCharsets.UTF_8); return new PrintWriter(writer, true) { @@ -336,4 +417,16 @@ public void close() { } }; } + + private static final class CompilationResult { + private final boolean success; + private final Map classes; + private final String diagnostics; + + private CompilationResult(boolean success, Map classes, String diagnostics) { + this.success = success; + this.classes = classes; + this.diagnostics = diagnostics; + } + } } diff --git a/src/test/java/net/openhft/compiler/CachedCompilerModuleClassLoaderReproTest.java b/src/test/java/net/openhft/compiler/CachedCompilerModuleClassLoaderReproTest.java new file mode 100644 index 0000000..d208705 --- /dev/null +++ b/src/test/java/net/openhft/compiler/CachedCompilerModuleClassLoaderReproTest.java @@ -0,0 +1,136 @@ +/* + * Copyright 2013-2025 chronicle.software; SPDX-License-Identifier: Apache-2.0 + */ +package net.openhft.compiler; + +import org.junit.Test; + +import javax.tools.JavaCompiler; +import javax.tools.StandardJavaFileManager; +import javax.tools.ToolProvider; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.Map; + +import static org.junit.Assert.*; + +public class CachedCompilerModuleClassLoaderReproTest { + + @Test + public void moduleLikeLoaderCanLoadClassAfterSuccessfulDefineClass() throws Exception { + ModuleLikeClassLoader loader = new ModuleLikeClassLoader(); + CachedCompiler compiler = new CachedCompiler(null, null); + + Class clazz = compiler.loadFromJava(loader, + "app.Generated", + "package app; public class Generated { public int value() { return 42; } }"); + + assertEquals("app.Generated", clazz.getName()); + assertSame(clazz, loader.loadClass("app.Generated")); + assertEquals(42, clazz.getDeclaredMethod("value").invoke(clazz.getDeclaredConstructor().newInstance())); + } + + @Test + public void compileFailureForLoaderOnlyDependencyReportsJavacDiagnostics() throws Exception { + ModuleLikeClassLoader loader = new ModuleLikeClassLoader(); + defineLoaderOnlyDependency(loader); + + assertSame("Sanity check: the supplied class loader can see the dependency", + loader.loadClass("app.Dto"), + Class.forName("app.Dto", false, loader)); + + CachedCompiler compiler = new CachedCompiler(null, null); + StringWriter diagnostics = new StringWriter(); + + ClassNotFoundException thrown = assertThrows(ClassNotFoundException.class, + () -> compiler.loadFromJava(loader, + "app.GeneratedUsesDto", + "package app; public class GeneratedUsesDto { app.Dto dto; }", + new PrintWriter(diagnostics))); + + assertTrue("Thrown exception should identify compilation failure: " + thrown.getMessage(), + thrown.getMessage().contains("Compilation failed for app.GeneratedUsesDto")); + assertTrue("Thrown exception should include javac missing-symbol diagnostics: " + thrown.getMessage(), + thrown.getMessage().contains("cannot find symbol")); + assertTrue("Thrown exception should include the dependency javac could not resolve: " + thrown.getMessage(), + thrown.getMessage().contains("Dto")); + assertNotNull("Thrown exception should carry a diagnostic cause", thrown.getCause()); + assertTrue("javac diagnostics should mention the dependency that the compiler could not resolve: " + + diagnostics, + diagnostics.toString().contains("Dto")); + assertNull("The generated class should not have been defined after javac failure", + loader.findLoaded("app.GeneratedUsesDto")); + } + + @Test + public void successfulCompileWithoutRequestedClassReportsMissingOutput() { + ModuleLikeClassLoader loader = new ModuleLikeClassLoader(); + CachedCompiler compiler = new CachedCompiler(null, null); + + ClassNotFoundException thrown = assertThrows(ClassNotFoundException.class, + () -> compiler.loadFromJava(loader, + "app.Expected", + "package app; class Different {}")); + + assertTrue("Thrown exception should identify the missing requested class: " + thrown.getMessage(), + thrown.getMessage().contains("Compilation did not produce requested class app.Expected")); + assertTrue("Thrown exception should identify the class javac actually produced: " + thrown.getMessage(), + thrown.getMessage().contains("app.Different")); + assertNotNull("Thrown exception should carry a diagnostic cause", thrown.getCause()); + assertNull("The requested class should not have been defined", + loader.findLoaded("app.Expected")); + } + + private static void defineLoaderOnlyDependency(ModuleLikeClassLoader loader) throws Exception { + JavaCompiler javac = ToolProvider.getSystemJavaCompiler(); + assertNotNull("System compiler required", javac); + + try (StandardJavaFileManager standardManager = javac.getStandardFileManager(null, null, null)) { + CachedCompiler bytecodeCompiler = new CachedCompiler(null, null); + MyJavaFileManager fileManager = new MyJavaFileManager(standardManager); + Map classes = bytecodeCompiler.compileFromJava( + "app.Dto", + "package app; public class Dto {}", + fileManager); + byte[] dtoBytes = classes.get("app.Dto"); + assertNotNull(dtoBytes); + CompilerUtils.defineClass(loader, "app.Dto", dtoBytes); + } + } + + private static final class ModuleLikeClassLoader extends ClassLoader { + private static final String APP_PREFIX = "app."; + + ModuleLikeClassLoader() { + super(CachedCompilerModuleClassLoaderReproTest.class.getClassLoader()); + } + + @Override + public Class loadClass(String name) throws ClassNotFoundException { + return loadClass(name, false); + } + + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + if (!name.startsWith(APP_PREFIX)) { + return super.loadClass(name, resolve); + } + return findClass(name, resolve); + } + + private Class findClass(String name, boolean resolve) throws ClassNotFoundException { + Class loaded = findLoadedClass(name); + if (loaded != null) { + if (resolve) { + resolveClass(loaded); + } + return loaded; + } + throw new ClassNotFoundException(name + " from [Module \"deployment.repro.war\" from Service Module Loader]"); + } + + Class findLoaded(String name) { + return findLoadedClass(name); + } + } +}