0xFableOrg/roll-op

Migrate away from subprocess.PIPE

norswap opened this issue · 1 comments

Our process spawning routine, lib.run (in libroll.py) has one mode where the output is redirected to another file. This uses the subprocess.PIPE mode. This is well documented as being problematic — for reasons that go beyond my current understanding (something something, buffers filling up — actually, here's a good explanation).

More concretely, this has manifest at least in the following issue for us: we get the "failed to promptly kill subprocess" messages killing the process with ctrl+c while some subprocesses are running.

Normally, the behaviour is that the signal sent by ctrl+c is caught, and all the subprocess are instructed to terminate. This message should be printed if we fail to terminate a subprocess in a timely manner. However, adding even large timeouts doesn't help. It turns out the subprocess is successfully terminated, the fact just doesn't bubble up to the subprocess library — again, here's the explanation, here's many people complaining about it.

We should deprecate use of the "fd" mode of lib.run in favour of the already existing stream mode which spawn a process to forward the output to a "stream", any old object with the write(self, text) and flush(self) methods, which should be the case of file objects.

So, the issue wasn't subprocess.PIPE at all. My suspicions came from this doc.

The problem is explained here (same as linked above) and it just seems that poll is hopelessly bugged in Python.

Fixing this required using the internal Popen._poll_internal(_deadstate="dead"). I see no other solution to check if a subprocess is alive (I guess we could record the PID and check using that).

Also the above is deadly confused, because the stream mode of lib.run relies on subprocess.PIPE and there's not much of an alternative (maybe fuck around with named pipes or implement a buffer file, but yikes).