Let system() and command-pipe children inherit the standard input - #577
Conversation
The children of system(cmd) and of a command input pipe
("cmd" | getline) now inherit the JVM's standard input when Jawk reads
the real standard input of the process (CLI runs), as POSIX requires
and as gawk, mawk, and BWK awk behave. Stdin filters and
terminal-aware commands such as "stty size" | getline now work.
Embedded executions bound to a custom Java input stream keep the
closed-stdin behavior: a Java stream cannot be lent to another OS
process, and handing over the host JVM's standard input would leak
input the embedder never gave to Jawk. Output pipes are unchanged: the
pipe itself remains the child's standard input.
Fixes #575
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d317b1c8a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| @Test | ||
| public void commandInputPipeChildReadsJawkStandardInput() throws Exception { | ||
| String output = runCliJvmWithStdin( |
There was a problem hiding this comment.
Both new inheritance tests execute AWK scripts through the private runCliJvmWithStdin helper instead of AwkTestSupport, bypassing the repository's explicit requirement for every new script-running unit test. Add the required subprocess capability to AwkTestSupport and invoke it here so setup, assertions, and temporary-file handling remain centralized.
AGENTS.md reference: AGENTS.md:L23-L29
Useful? React with 👍 / 👎.
| * input | ||
| */ | ||
| private boolean spawnedProcessInheritsStandardInput() { | ||
| return standardInput == System.in; |
There was a problem hiding this comment.
When an embedding application calls System.setIn(customStream) and then runs Jawk with that System.in—including through the default Cli constructor or JSR-223 fallback—this identity check returns true even though Redirect.INHERIT connects the child to the process's native descriptor 0, not the replacement Java stream. The child can therefore block on or consume unrelated host input instead of receiving closed stdin as promised for custom Java streams; native-stdin eligibility needs to be carried explicitly from the real CLI entry path.
Useful? React with 👍 / 👎.
| try (InputStream out = process.getInputStream(); InputStream err = process.getErrorStream()) { | ||
| copy(out, stdout); | ||
| copy(err, stderr); | ||
| } | ||
| assertTrue("CLI JVM did not terminate", process.waitFor(30, TimeUnit.SECONDS)); |
There was a problem hiding this comment.
If the spawned CLI hangs, or writes enough stderr to fill its pipe while stdout remains open, copy(out, stdout) blocks before waitFor(30, ...) is ever reached; the child can likewise block on the undrained stderr pipe. Thus the advertised timeout cannot prevent this test from hanging CI precisely when the subprocess behavior regresses. Drain both streams concurrently and apply the timeout before waiting for either stream to reach EOF, terminating the child on expiry.
Useful? React with 👍 / 👎.
- Capture the JVM's standard input when JRT is initialized and compare the bound input against that, so a stream installed later with System.setIn never qualifies for inheritance and the child's standard input fails closed instead of exposing the process's real descriptor 0. - Move the fresh-JVM CLI runner into AwkTestSupport, where every script-running test helper lives. - Redirect the child's standard output and error to files so no pipe can stall either side, enforce the timeout with waitFor alone, and kill the child when it expires. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb3a2393b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * replacement never matches, so the child's standard input stays closed | ||
| * instead of silently exposing the host's real descriptor 0. | ||
| */ | ||
| private static final InputStream PROCESS_STANDARD_INPUT = System.in; |
There was a problem hiding this comment.
When an embedding application calls System.setIn(customStream) before the first active use of JRT, this initializer captures that replacement as PROCESS_STANDARD_INPUT; the subsequent identity check therefore permits Redirect.INHERIT, exposing the JVM's native descriptor 0 to system() or command-pipe children instead of keeping their stdin closed, potentially causing them to consume unrelated host input or block. The fresh evidence relative to the earlier finding is that the new class-initialization-time capture still occurs after an embedder may replace System.in; native-stdin eligibility needs to be passed explicitly from the real process CLI entry path.
Useful? React with 👍 / 👎.
No capture of System.in can establish that Jawk reads the standard input of the JVM process: an embedder may replace the stream with System.setIn at any point, including before JRT initializes. The eligibility is now carried explicitly from Cli.main, the one caller that can vouch for it, through a JRT setter. Every other path — embedded API runs, tests, JSR-223 — leaves the flag unset, so the children of system() and of command pipes keep a closed standard input there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dd51dc371
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // this CLI reads is the standard input of the JVM itself, the one | ||
| // stream a spawned child can inherit; embedders and tests reach | ||
| // run() through other paths and never set this. | ||
| cli.processStandardInput = true; |
There was a problem hiding this comment.
When an embedder or test calls the public Cli.main after System.setIn(customStream), this unconditional flag makes system() and command-pipe children inherit the process's native descriptor 0 even though Jawk itself reads the replacement stream, so a child can block on or consume unrelated host input. The repository already invokes Cli.main programmatically in AwkTest.java, so main is not exclusively an OS launch path. The fresh evidence relative to the earlier finding is this new unconditional assignment; eligibility must only be enabled when the CLI input is actually backed by native stdin.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed that no capture or flag can survive a programmatic Cli.main call after System.setIn — and there is no supported Java-side check that can tell the launcher-installed System.in from a replacement (the fd0 backing of a stream is not observable without deep reflection into java.io). So the boundary is now drawn as a documented contract instead: Cli.main carries process semantics — it is what the jawk launch of the JVM runs, and its children may inherit the process's standard input, as POSIX requires of awk. Code that replaces System.in must go through create(), the constructors, or the Awk API, where spawned children always get a closed standard input. The one programmatic Cli.main call in the test suite (AwkTest.compileTuplesViaCLI) now goes through create() accordingly, so nothing in the repository calls main programmatically anymore.
There is no Java-side check that can distinguish the launcher-installed System.in from a replacement installed with System.setIn, so the standard-input inheritance declared by Cli.main is documented as part of its contract: main carries process semantics, and code that replaces System.in must use create(), the constructors, or the Awk API, where spawned children always get a closed standard input. The one programmatic Cli.main call in the test suite now goes through create() accordingly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: bertysentry <32521698+bertysentry@users.noreply.github.com>
Resolved the merge conflict and pushed merge commit |
Fixes #575
What
The children of
system(cmd)and of a command input pipe ("cmd" | getline) now inherit the JVM's standard input when Jawk reads the real standard input of the process — every CLI run — as POSIX requires and as gawk, mawk, and BWK awk behave:Terminal-aware commands work too:
"stty size" | getlinereaches the controlling terminal instead of failing withInappropriate ioctl for device, which is what aborted patsie75/awk-demo at startup.How
JRT.spawnProcessgains aninheritStandardInputflag mapping toProcessBuilder.Redirect.INHERIT. Thesystem()and command-input-pipe call sites passstandardInput == System.in; the output-pipe call site keeps the pipe as the child's standard input. Embedded executions bound to a custom Java input stream keep the closed-stdin behavior: a Java stream cannot be lent to another OS process, and handing over the host JVM's standard input would leak input the embedder never gave to Jawk.Tests
SpawnedProcessStdinTestcovers the three contracts: the pipe child and thesystem()child read Jawk's standard input across a real process boundary (CLI spawned in a fresh JVM with redirected stdin — the in-process builders cannot observe fd inheritance), and an embedded execution keeps the child's standard input closed. The scripts are passed via-fbecause Windows mangles embedded double quotes in inline command-line arguments.mvn verifypasses: all unit and compatibility tests, checkstyle, PMD, and SpotBugs clean. Documented injava-output.mdandbehavior-changes.md(Unreleased).🤖 Generated with Claude Code