[ruby-core:120463] [Ruby master Bug#20995] exception escapes block given to IO.popen("-") in child process

Issue #20995 has been updated by [e-mail] (Martin Dorey).


Domo arigato, Nobu-san, for agreeing that the previous behavior wasn't right and for such a quick fix.  I really like the idea of expressing how the code should behave with a unit test.  I was surprised by a couple of aspects of your test.  This is how I expected it to behave:
``` diff
martind@stormy:~/download/ruby$ git diff
...
diff --git a/test/ruby/test_process.rb b/test/ruby/test_process.rb
index 9c1836dffb..781fe77b7d 100644
--- a/test/ruby/test_process.rb
+++ b/test/ruby/test_process.rb
@@ -939,9 +939,10 @@ def test_popen_fork_ensure
           STDERR.reopen(STDOUT)
           raise "fooo"
         else
-          assert_empty io.read
+          assert_include(io.read, "fooo")
         end
       end
+      assert_not_predicate($?, :success?)
     rescue RuntimeError
       abort "[Bug #20995] should not reach here"
     end
martind@stormy:~/download/ruby$ 
```
I have very little idea whether this is safe but, with the following change, the above test passes:
``` diff
martind@stormy:~/download/ruby$ git diff
diff --git a/io.c b/io.c
index a78936488f..612c9caa40 100644
--- a/io.c
+++ b/io.c
@@ -8038,10 +8038,11 @@ popen_finish(VALUE port, VALUE klass)
     if (NIL_P(port)) {
         /* child */
         if (rb_block_given_p()) {
-            rb_protect(rb_yield, Qnil, NULL);
+            int state;
+            rb_protect(rb_yield, Qnil, &state);
             rb_io_flush(rb_ractor_stdout());
             rb_io_flush(rb_ractor_stderr());
-            _exit(0);
+            _exit(ruby_cleanup(state));
         }
         return Qnil;
     }
```
With that change, these ancestral behaviors:
```
martind@stormy:~/download/ruby$ ../ruby-3.3.4/ruby --disable-gems -we 'IO.popen("-") { |io| raise("a fuss") unless io; }; puts($?.inspect())'
-e:1:in `block in <main>': a fuss (RuntimeError)
	from -e:1:in `popen'
	from -e:1:in `<main>'
#<Process::Status: pid 120178 exit 1>
martind@stormy:~/download/ruby$ 
```
... which I think have positive value, are preserved:
```
martind@stormy:~/download/ruby$ ./ruby --disable-gems -we 'IO.popen("-") { |io| raise("a fuss") unless io; }; puts($?.inspect())'
-e:1:in 'block in <main>': a fuss (RuntimeError)
	from -e:1:in 'IO.popen'
	from -e:1:in '<main>'
#<Process::Status: pid 120135 exit 1>
martind@stormy:~/download/ruby$ 
```

----------------------------------------
Bug #20995: exception escapes block given to IO.popen("-") in child process
https://bugs.ruby-lang.org/issues/20995#change-111247

* Author: [e-mail] (Martin Dorey)
* Status: Closed
* Backport: 3.1: REQUIRED, 3.2: REQUIRED, 3.3: REQUIRED, 3.4: REQUIRED
----------------------------------------
I was surprised by the "ensure" being reached in the child process here:
``` ruby
martind@stormy:~/tmp/D161730$ cat repro.rb 
#!/usr/bin/ruby -w
parent = Process.pid()
[false, true].each() {
    |nauseous|
    $stderr.puts("#{nauseous ? "" : "not "}raising exception from child:")
    begin
        IO.popen("-") {
            |io|
            unless io
                if nauseous
                    raise("childish fit")
                end
            end
        }
    ensure
        $stderr.puts("in finalization block from #{Process.pid() == parent ? "parent" : "child"}")
        $stderr.puts()
    end
}
martind@stormy:~/tmp/D161730$ ~/download/ruby-3.3.4/ruby --version
ruby 3.3.4 (2024-07-09 revision be1089c8ec) [x86_64-linux]
martind@stormy:~/tmp/D161730$ ~/download/ruby-3.3.4/ruby -w repro.rb
not raising exception from child:
in finalization block from parent

raising exception from child:
in finalization block from child

repro.rb:11:in `block (2 levels) in <main>': childish fit (RuntimeError)
	from repro.rb:7:in `popen'
	from repro.rb:7:in `block in <main>'
	from repro.rb:3:in `each'
	from repro.rb:3:in `<main>'
in finalization block from parent

martind@stormy:~/tmp/D161730$ 
```
In the first iteration of the loop, we don't see the surprise - the child process exits when the block is completed.  Only when an exception is thrown, in the second iteration, does the control flow escape from the block.  It does so even for the SystemExit exception raised by exit(0) but not, as we see above, from just leaving the block (in which case perhaps we _exit(0) at https://github.com/ruby/ruby/blob/master/io.c#L8044).  My expectation was that the block would be executed in the child process much like the "main" part of a program, with the usual unhandled exception reporting, which we see demonstrated above, being invoked as soon as the exception propagated out the block, rather than first unwinding the callers.

I think the behavior is unchanged since the oldest version I was conveniently able to test, which was:

ruby 1.8.7 (2012-02-08 patchlevel 358) [x86_64-linux]

... so I didn't file this report as a Bug in case users have (accidentally?) come to depend on the behavior.  I didn't find any mention of this at eg https://ruby-doc.org/3.4.1/IO.html#method-c-popen but I didn't file this report as a Feature request for the documentation in case the documentation deliberately leaves open such possibilities.  I didn't even find anyone else puzzling over this behavior on eg StackOverflow, but I felt that I should report it for the benefit of the next poor sap to run into it, especially in case that's Future Me.  Could it be a case where a warning would be worth more than the trouble it causes?



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- [e-mail]
 To unsubscribe send an email to [e-mail]
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/