diff --git a/docs/about/changelog.md b/docs/about/changelog.md index 1927c9499..c654bb2fe 100644 --- a/docs/about/changelog.md +++ b/docs/about/changelog.md @@ -10,6 +10,9 @@ priorities and future plans. both execution backends, and align its notifier-loop refcount expectation with native Perl. +- Preserve buffered IPC::Open3 stdout and stderr until consumed before + reporting EOF, preventing IPC::Open3::Utils handler loss and pipe hangs. + - Fix parsing of dense Mo::Inline expressions that use `::` as a bareword. - Preserve UTF-8 HTML octets through HTML::Parser and no-op entity decoding, diff --git a/src/main/java/org/perlonjava/runtime/io/ProcessInputHandle.java b/src/main/java/org/perlonjava/runtime/io/ProcessInputHandle.java index e41bded2e..c477fece1 100644 --- a/src/main/java/org/perlonjava/runtime/io/ProcessInputHandle.java +++ b/src/main/java/org/perlonjava/runtime/io/ProcessInputHandle.java @@ -99,25 +99,15 @@ public RuntimeScalar fileno() { @Override public RuntimeScalar eof() { - if (isClosed) return RuntimeScalarCache.scalarTrue; - try { - // Check if stream has data available or is at EOF - if (isEOF) return RuntimeScalarCache.scalarTrue; - int available = inputStream.available(); - if (available > 0) return RuntimeScalarCache.scalarFalse; - - // Try to peek - if we get -1, it's EOF - inputStream.mark(1); - int ch = inputStream.read(); - if (ch == -1) { - isEOF = true; - return RuntimeScalarCache.scalarTrue; - } - inputStream.reset(); - return RuntimeScalarCache.scalarFalse; - } catch (IOException e) { - isEOF = true; - return RuntimeScalarCache.scalarTrue; + synchronized (readLock) { + // drainInput() is the sole reader of inputStream. Consulting the + // stream here races that thread and a one-byte "peek" can block + // while a child waits for stdin. More importantly, stream EOF is + // not Perl EOF until the bytes already drained into buffered have + // been returned to the caller. + return buffered.isEmpty() && isEOF + ? RuntimeScalarCache.scalarTrue + : RuntimeScalarCache.scalarFalse; } } @@ -167,4 +157,3 @@ public RuntimeScalar sysread(int length) { } } } - diff --git a/src/test/resources/unit/ipc_open3_buffered_eof.t b/src/test/resources/unit/ipc_open3_buffered_eof.t new file mode 100644 index 000000000..a0e0e90b0 --- /dev/null +++ b/src/test/resources/unit/ipc_open3_buffered_eof.t @@ -0,0 +1,70 @@ +#!/usr/bin/env perl +use strict; +use warnings; +use Test::More; +use IPC::Open3; +use IO::Select; +use Symbol qw(gensym); + +# Regression for #1263: ProcessInputHandle drains the Java stream in a reader +# thread. eof must inspect that synchronized buffer, not peek the Java stream, +# so callers such as IPC::Open3::Utils see queued stdout/stderr before EOF. +my ($stdin, $stdout, $stderr) = (undef, undef, gensym); +my @command = $^O eq 'MSWin32' + ? ('cmd.exe', '/v:on', '/c', 'set /p line=& echo out:!line!& echo err:!line! 1>&2') + : ('sh', '-c', 'read line; printf "out:%s\\n" "$line"; printf "err:%s\\n" "$line" >&2'); +my $pid = open3( + $stdin, $stdout, $stderr, @command +); + +print {$stdin} "payload\n"; +ok(close($stdin), 'closing stdin lets the child complete'); + +# Give the asynchronous readers time to transfer the child output into their +# private buffers. The process has exited, but buffered data is still readable. +select undef, undef, undef, 0.05; +ok(!eof($stdout), 'stdout with buffered child data is not EOF'); +ok(!eof($stderr), 'stderr with buffered child data is not EOF'); + +my $selector = IO::Select->new($stdout, $stderr); +my (%captured, $handler_calls, $short_circuit) = ((), 0, 0); + +READ_LOOP: +while (my @ready = $selector->can_read(2)) { + for my $fh (@ready) { + if (eof($fh)) { + $selector->remove($fh); + close($fh); + next; + } + + while (my $line = <$fh>) { + $handler_calls++; + $captured{fileno($fh)} .= $line; + $short_circuit = 1 if $line =~ /\Aout:payload\r?\n\z/; + last READ_LOOP if $short_circuit; + } + } +} + +ok($handler_calls, 'buffered data invokes the read handler'); +ok($short_circuit, 'handler can short-circuit after stdout is read'); +like($captured{fileno($stdout)} // '', qr/\Aout:payload\r?\n\z/, + 'stdout buffered data is preserved'); + +# Finish draining stderr after the short-circuit path, mirroring the lifecycle +# cleanup in IPC::Open3::Utils. +my $stderr_text = ''; +while (!eof($stderr)) { + my $line = <$stderr>; + $stderr_text .= $line if defined $line; +} +like($stderr_text, qr/\Aerr:/, + 'stderr buffered data is preserved'); + +close($stdout); +close($stderr); +is(waitpid($pid, 0), $pid, 'child is reaped after both pipes close'); +is($? >> 8, 0, 'child exits successfully'); + +done_testing;