From 64ff603c4c640561ce1d65275182c581d528dd5c Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Thu, 5 Jan 2023 08:37:18 +0000 Subject: [PATCH] Build: Fix "ValueError: I/O operation on closed file" Very occasionally, a build would fail with the above error. The root cause was that neither of the output streams for a process would be unregistered from the selector until both had reported EOF. This means that one of them could be requeued after it reported EOF, while the second was handled and (and the first unregistered/closed as a result). Then we would get around to handling the first one from the queue and bang. Solve this by unregistering the stream from the selector as soon as it reports EOF. Acomplish by refactoring _proc_deactivate() into _proc_stream_deactivate() + _proc_deactivate(). And also simplify some process shutdown code since popen is doing all the stream close() calls for us (proven with trace). Signed-off-by: Ryan Roberts --- shrinkwrap/utils/process.py | 50 ++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/shrinkwrap/utils/process.py b/shrinkwrap/utils/process.py index 0d88420..729bd94 100644 --- a/shrinkwrap/utils/process.py +++ b/shrinkwrap/utils/process.py @@ -100,13 +100,9 @@ class ProcessManager: data = self._read_nonblock(key.fileobj) if data == '': - assert(proc._active > 0) - proc._active -= 1 - if proc._active == 0: - self._proc_deactivate(proc) - else: - if self._handler: - self._handler(self, proc, data, streamid) + self._proc_stream_deactivate(proc, key.fileobj, streamid) + elif self._handler: + self._handler(self, proc, data, streamid) def _proc_activate(self, proc): cmd = runtime.mkcmd(proc.args, proc.interactive) @@ -156,40 +152,42 @@ class ProcessManager: if proc.run_to_end: self._active += 1 - def _proc_deactivate(self, proc, force=False): + def _proc_stream_deactivate(self, proc, stream, streamid, force=False): + if not stream: + return + + self._sel.unregister(stream) + + if streamid == STDOUT: + proc._stdout = None + if streamid == STDERR: + proc._stderr = None + + assert(proc._active > 0) + proc._active -= 1 + + if not force and proc._active == 0: + self._proc_deactivate(proc, force) + + def _proc_deactivate(self, proc, force): if not proc._popen: return if proc.run_to_end: self._active -= 1 - if proc._stdout: - self._sel.unregister(proc._stdout) - - if proc._stderr: - self._sel.unregister(proc._stderr) + proc._stdin = None + self._proc_stream_deactivate(proc, proc._stdout, STDOUT, force) + self._proc_stream_deactivate(proc, proc._stderr, STDERR, force) proc._popen.kill() try: proc._popen.communicate() except: pass - proc._popen.__exit__(None, None, None) retcode = None if force else proc._popen.poll() proc._popen = None - if proc._stdin: - proc._stdin.close() - proc._stdin = None - - if proc._stdout: - proc._stdout.close() - proc._stdout = None - - if proc._stderr: - proc._stderr.close() - proc._stderr = None - if self._terminate_handler: self._terminate_handler(self, proc, retcode) -- GitLab