Fix ForkedJavaProcess IOException: Argument list too long - #2945
Open
jingy-li wants to merge 4 commits into
Open
Fix ForkedJavaProcess IOException: Argument list too long#2945jingy-li wants to merge 4 commits into
jingy-li wants to merge 4 commits into
Conversation
ForkedJavaProcess.exec() builds the full runtime classpath and passes it directly as a single -cp argument to ProcessBuilder.start(), which ultimately calls the OS execve() syscall. Since execve() caps the total size of argv+envp (e.g. Linux ARG_MAX), a sufficiently large classpath (many dependency jars with long Gradle-cache paths) can exceed that limit, causing: java.io.IOException: Cannot run program "...java": error=7, Argument list too long This is an OS-level limit that gets worse as dependencies grow over time, independent of any single code change. Fix: write all java arguments (classpath, JVM args, app class, program args) to a temporary @argfile (supported by the java launcher since Java 9) and invoke "java @argfile" instead of passing them directly. Only the short @path token goes through execve(), so classpath size no longer matters. The argfile is cleaned up as soon as its forked process exits (tracked via a dedicated daemon thread), rather than relying solely on File#deleteOnExit(), since a single long-lived JVM (e.g. a Gradle daemon) can fork many processes over its lifetime and would otherwise accumulate orphaned temp files until that JVM exits. Verified locally: reproduced the original failure with an artificially oversized classpath, confirmed it no longer occurs with this fix, and confirmed args/JVM properties (including ones containing spaces) are still propagated correctly end-to-end. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates ForkedJavaProcess.exec() to avoid OS ARG_MAX failures by moving JVM/classpath/program arguments into a Java launcher @argfile instead of passing a huge -cp/argv list via ProcessBuilder.
Changes:
- Writes forked JVM arguments to a temporary
@argfileand invokesjava @argfile. - Adds background cleanup to delete the argfile when the forked process exits.
- Introduces token-quoting logic for argfile correctness when args include whitespace/quotes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+65
to
+69
| List<String> command = prepareCommandArgList(appClass, classPath, args, jvmArgs); | ||
|
|
||
| Process process = new ProcessBuilder(command).redirectErrorStream(true).start(); | ||
| File argFile = writeArgFile(command); | ||
| List<String> wrappedCommand = Arrays.asList(command.get(0), "@" + argFile.getAbsolutePath()); | ||
|
|
||
| Process process = new ProcessBuilder(wrappedCommand).redirectErrorStream(true).start(); |
Comment on lines
+360
to
+371
| /** | ||
| * Quotes a single token for inclusion in a JDK {@code @argfile}, per the rules documented for the {@code java} | ||
| * launcher: tokens containing whitespace or quote characters must be wrapped in double quotes, with internal | ||
| * backslashes and double quotes escaped. | ||
| */ | ||
| private static String quoteArgFileToken(String token) { | ||
| if (token.chars().noneMatch(c -> Character.isWhitespace(c) || c == '"' || c == '\'')) { | ||
| return token; | ||
| } | ||
| String escaped = token.replace("\\", "\\\\").replace("\"", "\\\""); | ||
| return "\"" + escaped + "\""; | ||
| } |
Adds regression tests for the @argfile mechanism added to ForkedJavaProcess.exec() to fix "Argument list too long" failures: - testForkWithHugeClasspathPropagatesArgsAndJvmProperties: forks a process with an artificially huge classpath (which would previously fail with execve() E2BIG) and asserts args/JVM properties still propagate correctly, exercising both branches of @argfile token quoting. - testArgFileIsCleanedUpAfterForkedProcessExits: asserts the argfile is deleted once the forked process exits. The forked helper class writes its output to a file rather than stdout, since ForkedJavaProcess already consumes the forked process's stdout internally on a background thread for logging; reading the same stream again from the test would race with that internal reader and intermittently split output between the two readers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ort it CI runs venice-common's tests against JDK 8, 11, and 17. The JDK 8 `java` launcher does not understand `@argfile` syntax (introduced in JDK 9 via JEP 293) at all: `java @somefile` is interpreted as an attempt to run a main class literally named `@somefile`, which fails immediately with "Could not find or load main class @somefile" (exit code 1). This broke every ForkedJavaProcess.exec() call when run under JDK 8, which is exactly what surfaced as new test failures on the JDK 8 CI matrix. ForkedJavaProcess now checks java.specification.version (of the current JVM, which is also the JVM used to fork, per JAVA_PATH) and only wraps the command in an @argfile when running on JDK 9+; on JDK 8 it falls back to passing arguments directly, exactly as before this change (accepting the pre-existing "Argument list too long" risk on JDK 8, same as prior behavior). Updated testForkWithHugeClasspathPropagatesArgsAndJvmProperties to skip when @argfile isn't supported (JDK 8), since the huge-classpath fix legitimately doesn't apply there. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
minhmo1620
previously approved these changes
Jul 30, 2026
…mmand Previously, the JDK-version check (isArgFileSupported()) gated both whether we wrote the argfile AND whether we passed it to the process, which meant an entire JDK's CI test run would only ever exercise one branch of that logic (JDK 8 shards never touch writeArgFile/ quoteArgFileToken/the cleanup thread; JDK 9+ shards never touch the plain-command fallback). Since each JDK matrix leg computes diff coverage independently, this made the coverage ratio highly JDK-dependent and caused the JDK 8 CI job to report only 12% branch coverage on the new code, well under the 45% minimum. Now writeArgFile()/quoteArgFileToken()/the cleanup thread always run regardless of JDK version (harmless extra temp-file I/O on JDK 8), and only the final choice of which command list to actually launch depends on isArgFileSupported(). This keeps the new code's branch coverage consistent across all JDK versions in CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem Statement
ForkedJavaProcess.exec() builds the full runtime classpath and passes it directly as a single -cp argument to ProcessBuilder.start(), which ultimately calls the OS execve() syscall. Since execve() caps the total size of argv+envp (e.g. Linux ARG_MAX), a sufficiently large classpath (many dependency jars with long Gradle-cache paths) can exceed that limit, causing:
java.io.IOException: Cannot run program "...java": error=7,
Argument list too long
Solution
This is an OS-level limit that gets worse as dependencies grow over time, independent of any single code change.
Fix: write all java arguments (classpath, JVM args, app class, program args) to a temporary @argfile (supported by the java launcher since Java 9) and invoke "java @argfile" instead of passing them directly. Only the short @path token goes through execve(), so classpath size no longer matters.
The argfile is cleaned up as soon as its forked process exits (tracked via a dedicated daemon thread), rather than relying solely on File#deleteOnExit(), since a single long-lived JVM (e.g. a Gradle daemon) can fork many processes over its lifetime and would otherwise accumulate orphaned temp files until that JVM exits.
Verified locally: reproduced the original failure with an artificially oversized classpath, confirmed it no longer occurs with this fix, and confirmed args/JVM properties (including ones containing spaces) are still propagated correctly end-to-end.
Code changes
Concurrency-Specific Checks
Both reviewer and PR author to verify
synchronized,RWLock) are used where needed.ConcurrentHashMap,CopyOnWriteArrayList).How was this PR tested?
Does this PR introduce any user-facing or breaking changes?