ResourceWarning: unclosed file <_io.BufferedWriter name=12>

[jiridanek]

https://github.com/skupperproject/skupper-router/runs/6243241095?check_suite_focus=true#step:9:1043

18: ::OneRouterTest::test_01_listen_error
18:   /usr/lib/python3.8/unittest/case.py:633: ResourceWarning: unclosed file <_io.BufferedWriter name=12>
18:     method()
18:   
18:   Object allocated at:
18:     File "/home/runner/.local/lib/python3.8/site-packages/pluggy/_manager.py", line 80
18:       return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
18:     File "/home/runner/.local/lib/python3.8/site-packages/pluggy/_callers.py", line 39
18:       res = hook_impl.function(*args)
18:     File "/home/runner/.local/lib/python3.8/site-packages/_pytest/main.py", line 347
18:       item.config.hook.pytest_runtest_protocol(item=item, nextitem=nextitem)
18:     File "/home/runner/.local/lib/python3.8/site-packages/pluggy/_hooks.py", line 265
18:       return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
18:     File "/home/runner/.local/lib/python3.8/site-packages/pluggy/_manager.py", line 80
18:       return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
18:     File "/home/runner/.local/lib/python3.8/site-packages/pluggy/_callers.py", line 39
18:       res = hook_impl.function(*args)
18:     File "/home/runner/.local/lib/python3.8/site-packages/_pytest/runner.py", line 111
18:       runtestprotocol(item, nextitem=nextitem)
18:     File "/home/runner/.local/lib/python3.8/site-packages/_pytest/runner.py", line 130
18:       reports.append(call_and_report(item, "call", log))
18:     File "/home/runner/.local/lib/python3.8/site-packages/_pytest/runner.py", line 219
18:       call = call_runtest_hook(item, when, **kwds)
18:     File "/home/runner/.local/lib/python3.8/site-packages/_pytest/runner.py", line 258
18:       return CallInfo.from_call(
18:     File "/home/runner/.local/lib/python3.8/site-packages/_pytest/runner.py", line 338
18:       result: Optional[TResult] = func()
18:     File "/home/runner/.local/lib/python3.8/site-packages/_pytest/runner.py", line 259
18:       lambda: ihook(item=item, **kwds), when=when, reraise=reraise
18:     File "/home/runner/.local/lib/python3.8/site-packages/pluggy/_hooks.py", line 265
18:       return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
18:     File "/home/runner/.local/lib/python3.8/site-packages/pluggy/_manager.py", line 80
18:       return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
18:     File "/home/runner/.local/lib/python3.8/site-packages/pluggy/_callers.py", line 39
18:       res = hook_impl.function(*args)
18:     File "/home/runner/.local/lib/python3.8/site-packages/_pytest/runner.py", line 166
18:       item.runtest()
18:     File "/home/runner/.local/lib/python3.8/site-packages/_pytest/unittest.py", line 327
18:       self._testcase(result=self)  # type: ignore[arg-type]
18:     File "/usr/lib/python3.8/unittest/case.py", line 736
18:       return self.run(*args, **kwds)
18:     File "/usr/lib/python3.8/unittest/case.py", line 676
18:       self._callTestMethod(testMethod)
18:     File "/usr/lib/python3.8/unittest/case.py", line 633
18:       method()

The stacktrace above is useless, but the test itself is simple and there appears only one place which deals with fd's, the subprocess call.

skupper-router/tests/system_tests_one_router.py

Lines 409 to 415 in 4dac887

	def test_01_listen_error(self):
	# Make sure a router exits if a initial listener fails, doesn't hang.
	config = Qdrouterd.Config([
	('router', {'mode': 'standalone', 'id': 'bad'}),
	('listener', {'port': OneRouterTest.listen_port})])
	r = Qdrouterd(name="expect_fail", config=config, wait=False)
	self.assertEqual(1, r.wait())

Looking at the code, it looks sufficient to me. The code calls subprocess.wait, which I always thought is enough. And, trying with /usr/bin/python-debug, it appears it is enough. Unless the subprocess redirects outputs, but qdrouterd does not do anything like the following

import subprocess

p = subprocess.Popen(["bash", "-c", "echo baf"], stdout=subprocess.PIPE)
p.wait()

# produces
# $ python-debug -Xdev scratches/scratch_8.py
# sys:1: ResourceWarning: unclosed file <_io.BufferedReader name=3>

But Process does!

skupper-router/tests/system_test.py

Lines 300 to 310 in fdfd076

	kwargs.setdefault('stdin', subprocess.PIPE)
	with open(self.outfile + '.out', 'w') as out:
	kwargs.setdefault('stdout', out)
	kwargs.setdefault('stderr', subprocess.STDOUT)
	try:
	super(Process, self).__init__(args, **kwargs)
	with open(self.outfile + '.cmd', 'w') as f:
	f.write("%s\npid=%s\n" % (' '.join(args), self.pid))
	except Exception as e:
	raise Exception("subprocess.Popen(%s, %s) failed: %s: %s" %
	(args, kwargs, type(e).__name__, e))

I am surprised one can get away with the with since the fd clearly escapes out of the with's scope...

The subprocess.Popen#__exit__ in the standard library is implemented like this

    def __exit__(self, exc_type, value, traceback):
        if self.stdout:
            self.stdout.close()
        if self.stderr:
            self.stderr.close()
        try:  # Flushing a BufferedWriter may raise an error
            if self.stdin:
                self.stdin.close()
        finally:
            if exc_type == KeyboardInterrupt:
                # https://bugs.python.org/issue25942
                # In the case of a KeyboardInterrupt we assume the SIGINT
                # was also already sent to our child processes.  We can't
                # block indefinitely as that is not user friendly.
                # If we have not already waited a brief amount of time in
                # an interrupted .wait() or .communicate() call, do so here
                # for consistency.
                if self._sigint_wait_secs > 0:
                    try:
                        self._wait(timeout=self._sigint_wait_secs)
                    except TimeoutExpired:
                        pass
                self._sigint_wait_secs = 0  # Note that this has been done.
                return  # resume the KeyboardInterrupt

            # Wait for the process to terminate, to avoid zombies.
            self.wait()

[jiridanek]

my_f = None

with open('file', 'w') as f:
    my_f = f

f.write("aaaa\n")
f.flush()

  File "scratches/scratch_8.py", line 8, in <module>
    f.write("aaaa\n")
ValueError: I/O operation on closed file.

The trick is that subprocess forks a new process, so closing file in parent does not influence the fd in child, of course. At least that is my guess. In any case, the following is what router tests do and it works just fine (and does not produce warnings in python-debug -Xdev, which is the most important thing. Useless warnings should not be printed so that we can see any new and interesting ones

import subprocess

with open('file', 'w') as f:
    p = subprocess.Popen(["bash", "-c", "sleep 1; echo baf"], stdout=f, stderr=subprocess.STDOUT)

p.wait()