Race condition when outputting config during parallel execution of tests in agent mode

[lazar-mitrovic]

At the moment, when native-image-agent is injected into JVM processes that are spawned for test executions we are specifying its output directory as a fixed path.
In Maven plugin this is done here:

native-build-tools/native-maven-plugin/src/main/java/org/graalvm/buildtools/maven/NativeExtension.java

Lines 86 to 90 in 5ac47c6

	static String buildAgentArgument(String baseDir, Context context, List<String> agentOptions) {
	List<String> options = new ArrayList<>(agentOptions);
	options.add("config-output-dir=" + agentOutputDirectoryFor(baseDir, context));
	return "-agentlib:native-image-agent=" + String.join(",", options);
	}

In Gradle:

native-build-tools/native-gradle-plugin/src/main/java/org/graalvm/buildtools/gradle/internal/AgentCommandLineProvider.java

Lines 77 to 93 in b14f07b

	@Override
	public Iterable<String> asArguments() {
	if (getEnabled().get()) {
	File outputDir = getOutputDirectory().getAsFile().get();
	List<String> agentOptions = new ArrayList<>(getAgentOptions().getOrElse(Collections.emptyList()));
	if (agentOptions.stream().map(s -> s.split("=")[0]).anyMatch(s -> s.contains("config-output-dir"))) {
	throw new IllegalStateException("config-output-dir cannot be supplied as an agent option");
	}
	agentOptions.add("config-output-dir=" + outputDir.getAbsolutePath());
	return Arrays.asList(
	"-agentlib:native-image-agent=" + String.join(",", agentOptions),
	"-Dorg.graalvm.nativeimage.imagecode=agent"
	);
	}
	return Collections.emptyList();
	}
	}

An issue that arises from this approach is that each parallel test execution overwrites older configuration.

Trivial solution for this would be change the invocation parameter to config-merge-dir, but that would (AFAIK) require that tests are being run sequentially which would hinder performance significantly. Potentially we could hack the agent itself to wait on some kind of file lock if user opted-in to that, but that won't land in a release anytime soon.

Proper solution would be to use {pid} and {datetime} placeholders for output directory (as described in this section of the documentation)

export JAVA_TOOL_OPTIONS="-agentlib:native-image-agent=config-output-dir=/path/to/config-output-dir-{pid}-{datetime}/"

and then merge configuration using native-image-configure tool after test execution is done.

Unfortunately, as documentation points out:

Note: The Native Image Configure Tool is only available if native-image is built via mx. 
This configuration tool is not part of any GraalVM distribution by default.

We might be able to provide ConfigurationTool as a separate Maven artifact, or re-implement it entirely using off-the-shelf JSON libraries (like what we are doing in Gradle currently in order to filter out Gradle internal classes that are leaking into configuration).
cc: @peter-hofer

Another consideration is whether we should switch to JAVA_TOOL_OPTIONS environment variable for agent configuration or is the current approach sufficient (e.g. is every JVM process that we spawn configured correctly?). On pro side that would massively simplify Maven configuration (that is currently manipulating xml dom to inject nodes), but this option might (?) be Hotspot specific and could as such prevent things like running tests using Espresso further down the line.

cc: @melix @alvarosanchez @vjovanov

[peter-hofer]

native-image-configure is not included as a native image, but it should be included as a shell/batch script now. If not, you can build it via native-image --macro:native-image-configure-launcher. (Either way, the docs should be updated to reflect that)

[melix]

Let me think about it, but I don't think we should rely on JAVA_TOOL_OPTIONS: that's a poor man's solution which is transparent to build tools: there's no way they can know what's going on. Whatever solution we come up with must make sure that the build tool can effectively determine what agent files were generated during a particular invocation. Then if they have to merge them, so be it. I would very much prefer a Java API that we can call (or a Java CLI) than a shell script, though, as it's much less likely to break on "unusual" environments.

[peter-hofer]

You don't need to use JAVA_TOOL_OPTIONS to use the placeholders.

[lazar-mitrovic]

That makes sense. So the process should look something like this:

1. Amend agent invocation

Path tmpDir = outputDir.resolve("tmp").toAbsolutePath();
// Ensure that tmpDir is empty
// ...
Path template = tmpDir.resolve("{pid}-{datetime}")
agentOptions.add("config-output-dir=" + template);
// ...

2. Add a new step that merges configuration after JVM execution

// Generate `native-image-configuration` invocation file
// that contains all generated directories. This is necessary
// if we want to prevent long command-line errors on Windows.
String inputDirs = Arrays.asList(tmpDir.toFile().list())
              .stream()
              .map(dir -> "--input-dir=" + tmpDir.resolve(dir).toAbsolutePath())
              .collect(Collectors.joining("\n"));
Path commandFile = tmpDir.resolve("nic-command.txt");

// Dump inputDirs to commandFile 
// ...

Path nic = Paths.get(graalHome)
        .resolve("bin")
        .resolve(NATIVE_IMAGE_CONFIGURE); // $GRAALVM_HOME/bin/native-image-configure

// Check if nic exists, and if not run `gu install native-image`
// ...

List<String> command = new ArrayList<>(Arrays.asList(
        nic.toString(),
        commandFile.toString(),
        "--exclude-classes=<class-pattern>", // useful for Gradle
        "--output-dir=" + outputDir.toAbsolutePath()
));
ProcessBuilder processBuilder = new ProcessBuilder(command);
processBuilder.directory(new File(project.getBuild().getDirectory()));
processBuilder.inheritIO();

// Delete tmpDir and its contents
// ...

#190 should probably be merged beforehand.

[melix]

I am looking into implementing this with Gradle. A couple questions:

• is the native-image --macro:native-image-configure-launcher valid on any GraalVM installation? In particular, I'm interested in knowing if it would work with GraalVM CE, GraalVM Enterprise Edition in different versions, as well as 3rd party GraalVM SDKs like BellSoft
• what is the native-image-configure-launcher script extension under Windows?

[melix]

So I have something which works with Gradle and GraalVM CE, but unfortunately this breaks with Liberica:

Successfully started process 'command '/home/cchampeau/.sdkman/candidates/java/21.3.0.r11-nik/bin/native-image''
Error: Unknown name in option specification: macro:native-image-configure-launcher
Available macro options are:
    --macro:polyglot-launcher
    --macro:gu-launcher
    --macro:native-image-agent-library
    --macro:truffle
    --macro:graalvm-native-binutil-launcher
    --macro:graalvm-native-clang-launcher
    --macro:graalvm-native-ld-launcher
    --macro:native-image-diagnostics-agent-library
    --macro:js-launcher
    --macro:graalvm-native-clang++-launcher
    --macro:lli-launcher
    --macro:polyglot-library
    --macro:native-image-launcher

[vjovanov]

You can always use the one from GraalVM.

[melix]

Not sure I understand: are you suggesting that the plugin bundles a tool which is generated by GraalVM CE? I'm a little nervous at the idea of doing this, not knowing what the target platform is.

[lazar-mitrovic]

@melix
Don't use native-image --macro:native-image-configure-launcher, use $GRAALVM_HOME/bin/native-image-configure
native-image-configure is built the same way as native-image, so extension on Windows is .cmd

[melix]

native-image --macro:native-image-configure-launcher is used to generate the native-image-configure script. It's absent by default on installs, as per the docs. Unfortunately, this doesn't work for Liberica, which misses the script and the macro.

[melix]

I guess alternatively we could merge ourselves? Is the merging process documented?