Subject: Re: IPC::Run accepts bug reports

From: Alexander Lakhin <[email-masked]>
Date: Fri, 4 Oct 2024 14:00:00 +0300

Hello Noah,

16.06.2024 02:48, Noah Misch wrote:
> I don't see in https://github.com/cpan-authors/IPC-Run/issues anything
> affecting PostgreSQL.  If you know of IPC::Run defects, please report them.
> If I knew of an IPC::Run defect affecting PostgreSQL, I likely would work on
> it before absurdity like https://github.com/cpan-authors/IPC-Run/issues/175
> NetBSD-10-specific behavior coping.

It looks like a recent indri failure [1] revealed one more IPC::Run
anomaly. The failure log contains:
# Running: pgbench -n -t 1 -Dfoo=bla -Dnull=null -Dtrue=true -Done=1 -Dzero=0.0 -Dbadtrue=trueXXX 
-Dmaxint=9223372036854775807 -Dminint=-9223372036854775808 -M prepared -f 
/Users/buildfarm/bf-data/HEAD/pgsql.build/src/bin/pgbench/tmp_check/t_001_pgbench_with_server_main_data/001_pgbench_error_sleep_undefined_variable
[22:38:14.887](0.014s) ok 362 - pgbench script error: sleep undefined variable status (got 2 vs expected 2)
[22:38:14.887](0.000s) ok 363 - pgbench script error: sleep undefined variable stdout /(?^:processed: 0/1)/
[22:38:14.887](0.000s) not ok 364 - pgbench script error: sleep undefined variable stderr /(?^:sleep: undefined variable)/
[22:38:14.887](0.000s)
[22:38:14.887](0.000s) #   Failed test 'pgbench script error: sleep undefined variable stderr /(?^:sleep: undefined 
variable)/'
#   at t/001_pgbench_with_server.pl line 1242.
[22:38:14.887](0.000s) #                   ''
#     doesn't match '(?^:sleep: undefined variable)'

So the pgbench process exited as expected, stdout contained expected
string, but stderr happened to be empty.

Maybe such behavior is specific to macOS, and even on indri it's the
only failure of that ilk out of 2000+ runs (and I couldn't reproduce
this in a Linux VM), but I find this place in IPC::Run suspicious:
sub _read {
...
     my $r = POSIX::read( $_[0], $s, 10_000 );
     croak "$!: read( $_[0] )" if not($r) and !$!{EINTR};

That is, EINTR kind of recognized as an expected error, but there is no
retry in this case. Thus, with the following modification, which simulates
read() failed with EINTR:
  sub _read {
      confess 'undef' unless defined $_[0];
      my $s = '';
-    my $r = POSIX::read( $_[0], $s, 10_000 );
+    my $r;
+if (int(rand(100)) == 0)
+{
+   $r = 0;  $! = Errno::EINTR;
+}
+else
+{
+    $r = POSIX::read( $_[0], $s, 10_000 );
+}
      croak "$!: read( $_[0] )" if not($r) and !$!{EINTR};

I can see failures like the one in question when running that test.

Perhaps, I could reproduce the issue with a program, that sends signals
to running (pgbench) processes (and thus interrupts read()), if it makes
sense.

What do you think?

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=indri&dt=2024-10-02%2002%3A34%3A16

Best regards,
Alexander

---

From: Noah Misch <[email-masked]>
Date: Fri, 4 Oct 2024 11:57:48 -0700

> I can see failures like the one in question when running that test.

That makes sense.  Would you file this at
https://github.com/cpan-authors/IPC-Run/issues?  I suppose that code should
become roughly:

  do { $r = POSIX::read(...) } while (!defined($r) && $!{EINTR});
  croak ... unless defined($r);

> to running (pgbench) processes (and thus interrupts read()), if it makes

That should work.  Best would be an IPC::Run test case, like the existing
t/eintr.t that makes select() report EINTR.  That said, IPC::Run could adopt
the retry without a test.