worker: flush captured stderr on abnormal exit, gate trace markers behind env var#1841
Open
gergelyfabian wants to merge 1 commit into
Open
Conversation
…hind env var The persistent-worker loop redirects System.out / System.err into an in-memory buffer (outStream) so the bytes ride back in WorkResponse.output. When the JVM exits before the next WorkResponse is written — direct System.exit(N) from inside work() (scalac's reporter, a compiler plugin, any wrapper that bails via Runtime.exit), uncaught Throwable unwinding the main thread, signal-induced shutdown that still runs hooks — the captured buffer dies in memory and the worker server receives nothing but an EOF on the codec. SIGKILL, native JVM crashes, and Runtime.halt are unavoidably uncovered. Snapshot System.err into a `realStdErr` local before any setErr() call and install a JVM shutdown hook that, on every termination path that runs hooks, flushes the in-flight outStream onto realStdErr (still wired to the worker server's pipe). The normal-completion path resets outStream after every request, so a clean stdin-EOF shutdown leaves the buffer empty; the hook is removed from the runtime in the outer finally on normal exit. Track in-flight argv and a completed-request counter so the abnormal-exit diagnostic can name the action that was running. Captured tail is bounded to 64 KB (last bytes wins; the failure-causing output is typically at the end), preventing a runaway scalac from flooding the worker server's stderr ring buffer. Per-request start/end markers and lifecycle banners are gated behind the RULES_SCALA_WORKER_TRACE env var — off by default, since the shutdown-hook diagnostic already names the in-flight argv. Set to a truthy value (e.g. RULES_SCALA_WORKER_TRACE=1) when debugging worker behavior. Tests cover the summarizeArgv helper and exercise the shutdown-hook / trace-mode paths via a forked JVM subprocess (testing them in-process would require taking the test JVM down). The ephemeral (non-persistent) path is unchanged; it never hijacks the streams and so never needed the recovery path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
gergelyfabian
requested review from
WojciechMazur,
mbland and
simuons
as code owners
June 18, 2026 05:05
Contributor
Author
|
I'm not sure this should land in master, just sending for the record. Very helpful for debugging if anything goes south in error handling. |
PawelLipski
changed the title
Collaborator
|
Hey, thanks for the change - let me add some comments on possible test improvements/additions, otherwise good to go |
PawelLipski
reviewed
Jul 4, 2026
| AtomicReference<String> inFlightArgv, | ||
| AtomicLong requestsCompleted, | ||
| String prefix) { | ||
| final int maxTailBytes = 64 * 1024; |
Collaborator
There was a problem hiding this comment.
Consider adding a test for this truncation logic as well (AFAICS not covered yet)
PawelLipski
reviewed
Jul 4, 2026
| String inFlight = inFlightArgv.get(); | ||
|
|
||
| realStdErr.println(); | ||
| realStdErr.println(prefix + "abnormal-exit diagnostic"); |
Collaborator
There was a problem hiding this comment.
Is there a test for happy path - hook not run, no abnormal-exit diagnostic printed out if the worker shuts down successfully? in case the logic around removeShutdownHook is ever removed
PawelLipski
reviewed
Jul 4, 2026
| } | ||
|
|
||
| /** | ||
| * Installs a JVM shutdown hook that, on abnormal exit, flushes the persistent worker's captured |
Collaborator
There was a problem hiding this comment.
Maybe explain what's the mechanism for making sure it only fires on an abnormal exit (removeShutdownHook, I suppose)
PawelLipski
approved these changes
Jul 4, 2026
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
2 participants
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.
The persistent-worker loop redirects System.out / System.err into an in-memory buffer (outStream) so the bytes ride back in WorkResponse.output. When the JVM exits before the next WorkResponse is written — direct System.exit(N) from inside work() (scalac's reporter, a compiler plugin, any wrapper that bails via Runtime.exit), uncaught Throwable unwinding the main thread, signal-induced shutdown that still runs hooks — the captured buffer dies in memory and the worker server receives nothing but an EOF on the codec. SIGKILL, native JVM crashes, and Runtime.halt are unavoidably uncovered.
Snapshot System.err into a
realStdErrlocal before any setErr() call and install a JVM shutdown hook that, on every termination path that runs hooks, flushes the in-flight outStream onto realStdErr (still wired to the worker server's pipe). The normal-completion path resets outStream after every request, so a clean stdin-EOF shutdown leaves the buffer empty; the hook is removed from the runtime in the outer finally on normal exit. Track in-flight argv and a completed-request counter so the abnormal-exit diagnostic can name the action that was running. Captured tail is bounded to 64 KB (last bytes wins; the failure-causing output is typically at the end), preventing a runaway scalac from flooding the worker server's stderr ring buffer.Per-request start/end markers and lifecycle banners are gated behind the RULES_SCALA_WORKER_TRACE env var — off by default, since the shutdown-hook diagnostic already names the in-flight argv. Set to a truthy value (e.g. RULES_SCALA_WORKER_TRACE=1) when debugging worker behavior.
Tests cover the summarizeArgv helper and exercise the shutdown-hook / trace-mode paths via a forked JVM subprocess (testing them in-process would require taking the test JVM down).
The ephemeral (non-persistent) path is unchanged; it never hijacks the streams and so never needed the recovery path.
Motivation
Used to debug #1840.