Popen2IO: Fix "Bad file descriptor" error

Fix init_popen_io to leave behind a sane state (0 and 1 file
descriptors open), in order to prevent "Bad file descriptor"
errors. Also fix test_stdouterrin_setnull to restore stdin
state, while relying on capfd to do this for stdout and stderr.

The "Bad file descriptor" error doesn't trigger reliably
because it's triggered by garbage collection of the Popen2IO
instance returned from init_popen_io as shown in this job:

https://github.com/pytest-dev/execnet/actions/runs/7955155978/job/21716386705?pr=243

=================================== FAILURES ===================================
__________________ test_stdouterrin_setnull[main_thread_only] __________________

execmodel = <ExecModel 'main_thread_only'>
capfd = <_pytest.capture.CaptureFixture object at 0x7fba8c333990>

    @pytest.mark.skipif("not hasattr(os, 'dup')")
    def test_stdouterrin_setnull(execmodel, capfd):
        gateway_base.init_popen_io(execmodel)
        os.write(1, b"hello")
>       os.read(0, 1)
E       OSError: [Errno 9] Bad file descriptor

testing/test_basics.py:254: OSError

Author: zmedico

src/execnet/gateway_base.py

@@ -61,7 +61,7 @@ def sleep(self, delay):
         raise NotImplementedError()
 
     @abc.abstractmethod
-    def fdopen(self, fd, mode, bufsize=1):
+    def fdopen(self, fd, mode, bufsize=1, closefd=True):
         raise NotImplementedError()
 
     @abc.abstractmethod
@@ -113,10 +113,10 @@ def start(self, func, args=()):
 
         return _thread.start_new_thread(func, args)
 
-    def fdopen(self, fd, mode, bufsize=1):
+    def fdopen(self, fd, mode, bufsize=1, closefd=True):
         import os
 
-        return os.fdopen(fd, mode, bufsize, encoding="utf-8")
+        return os.fdopen(fd, mode, bufsize, encoding="utf-8", closefd=closefd)
 
     def Lock(self):
         import threading
@@ -174,10 +174,10 @@ def start(self, func, args=()):
 
         return eventlet.spawn_n(func, *args)
 
-    def fdopen(self, fd, mode, bufsize=1):
+    def fdopen(self, fd, mode, bufsize=1, closefd=True):
         import eventlet.green.os
 
-        return eventlet.green.os.fdopen(fd, mode, bufsize)
+        return eventlet.green.os.fdopen(fd, mode, bufsize, closefd=closefd)
 
     def Lock(self):
         import eventlet.green.threading
@@ -231,11 +231,11 @@ def start(self, func, args=()):
 
         return gevent.spawn(func, *args)
 
-    def fdopen(self, fd, mode, bufsize=1):
+    def fdopen(self, fd, mode, bufsize=1, closefd=True):
         # XXX
         import gevent.fileobject
 
-        return gevent.fileobject.FileObjectThread(fd, mode, bufsize)
+        return gevent.fileobject.FileObjectThread(fd, mode, bufsize, closefd=closefd)
 
     def Lock(self):
         import gevent.lock
@@ -1682,8 +1682,10 @@ def init_popen_io(execmodel):
             os.dup2(fd, 2)
         os.close(fd)
         io = Popen2IO(stdout, stdin, execmodel)
-        sys.stdin = execmodel.fdopen(0, "r", 1)
-        sys.stdout = execmodel.fdopen(1, "w", 1)
+        # Use closefd=False since 0 and 1 are shared with
+        # sys.__stdin__ and sys.__stdout__.
+        sys.stdin = execmodel.fdopen(0, "r", 1, closefd=False)
+        sys.stdout = execmodel.fdopen(1, "w", 1, closefd=False)
     return io
 
 

testing/test_basics.py

@@ -249,12 +249,25 @@ class Arg:
 
 @pytest.mark.skipif("not hasattr(os, 'dup')")
 def test_stdouterrin_setnull(execmodel, capfd):
-    gateway_base.init_popen_io(execmodel)
-    os.write(1, b"hello")
-    os.read(0, 1)
-    out, err = capfd.readouterr()
-    assert not out
-    assert not err
+    # Backup and restore stdin state, and rely on capfd to handle
+    # this for stdout and stderr.
+    orig_stdin = sys.stdin
+    orig_stdin_fd = os.dup(0)
+    try:
+        # The returned Popen2IO instance can be garbage collected
+        # prematurely since we don't hold a reference here, but we
+        # tolerate this because it is intended to leave behind a
+        # sane state afterwards.
+        gateway_base.init_popen_io(execmodel)
+        os.write(1, b"hello")
+        os.read(0, 1)
+        out, err = capfd.readouterr()
+        assert not out
+        assert not err
+    finally:
+        sys.stdin = orig_stdin
+        os.dup2(orig_stdin_fd, 0)
+        os.close(orig_stdin_fd)
 
 
 class PseudoChannel: