Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 60 additions & 5 deletions src/main/java/net/openhft/compiler/CachedCompiler.java
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,13 @@
@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<? extends JavaFileObject> compilationUnits;
if (sourceDir != null) {
Expand All @@ -186,12 +193,15 @@
javaFileObjects.put(className, new JavaSourceFromString(className, javaCode));
compilationUnits = new ArrayList<>(javaFileObjects.values()); // To prevent CME from compiler code
}
StringBuilder diagnostics = new StringBuilder();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This StringBuilder is captured in the DiagnosticListener lambda below, but if javac calls report() on multiple threads, then StringBuilder.append() will not be thread safe. Might be better to use StringBuffer or else explicit synchronisation, unless we can guarantee this will always be single-threaded ?

Copy link
Copy Markdown
Contributor

@benbonavia benbonavia May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compile is done in this thread from the call() method, so I don't think there foresee any concurrency issues

// reuse the same file manager to allow caching of jar files
boolean ok = s_compiler.getTask(writer, fileManager, new DiagnosticListener<JavaFileObject>() {
@Override
public void report(Diagnostic<? extends JavaFileObject> 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();
Expand All @@ -202,11 +212,11 @@
javaFileObjects.remove(className);

// nothing to return due to compiler error
return Collections.emptyMap();
return new CompilationResult(false, Collections.emptyMap(), diagnostics.toString());
} else {
Map<String, byte[]> result = fileManager.getAllBuffers();

return result;
return new CompilationResult(true, result, diagnostics.toString());
}
}

Expand All @@ -222,7 +232,7 @@
* @return the loaded class instance
* @throws ClassNotFoundException if definition fails
*/
public Class<?> loadFromJava(@NotNull ClassLoader classLoader,

Check failure on line 235 in src/main/java/net/openhft/compiler/CachedCompiler.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 18 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=OpenHFT_Java-Runtime-Compiler&issues=AZ4ClVMW78PzteU-X-vm&open=AZ4ClVMW78PzteU-X-vm&pullRequest=180
@NotNull String className,
@NotNull String javaCode,
@Nullable PrintWriter writer) throws ClassNotFoundException {
Expand All @@ -245,7 +255,16 @@
fileManager = getFileManager(standardJavaFileManager);
fileManagerMap.put(classLoader, fileManager);
}
final Map<String, byte[]> compiled = compileFromJava(className, javaCode, printWriter, fileManager);
final CompilationResult compilation = compileFromJavaResult(className, javaCode, printWriter, fileManager);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth having the compilation result examined from within the compileFromJavaResult() method to reduce complexity in this method?

Also this way, changing of the method signature may not be required as it can still return the Map<String, byte[]> of the compiled classes if it is a success and the className exists in the map of compiled (as its provided to the compileFromJava() method. Then to allow for diagnostics to be used, you can pass in the StringBuilder on the call - this can determine whether the caller actually cares about diagnostics and add the detail if it's asked for

if (!compilation.success) {
throw compilationFailedException(className, compilation.diagnostics);
}

final Map<String, byte[]> compiled = compilation.classes;
if (!compiled.containsKey(className)) {
throw missingCompiledClassException(className, compiled.keySet(), compilation.diagnostics);
}

for (Map.Entry<String, byte[]> entry : compiled.entrySet()) {
String className2 = entry.getKey();
validateClassName(className2);
Expand Down Expand Up @@ -275,7 +294,10 @@
}
}
synchronized (loadedClassesMap) {
loadedClasses.put(className, clazz = classLoader.loadClass(className));
clazz = loadedClasses.get(className);
}
if (clazz == null) {
throw missingCompiledClassException(className, compiled.keySet(), compilation.diagnostics);
}
return clazz;
}
Expand Down Expand Up @@ -327,6 +349,27 @@
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<String> 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) {
Expand All @@ -336,4 +379,16 @@
}
};
}

private static final class CompilationResult {
private final boolean success;
private final Map<String, byte[]> classes;
private final String diagnostics;

private CompilationResult(boolean success, Map<String, byte[]> classes, String diagnostics) {
this.success = success;
this.classes = classes;
this.diagnostics = diagnostics;
}
}
}
Original file line number Diff line number Diff line change
@@ -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<String, byte[]> 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);
}
}
}