seeing-things/track

Don't use `subprocess.run()` to run `track` programs

Closed this issue · 0 comments

It turns out that using the subprocess.run() convenience function has an extremely inconvenient side effect: If any exception is raised while that function is blocked waiting for a subprocess to complete, such as SystemExit or KeyboardInterrupt, it will just unceremoniously murder the subprocess by sending it SIGKILL. This gives the subprocess no opportunity to shut down gracefully: No context managers will clean up resources, any active slews will just continue forever, other child processes of that subprocess might be left as zombies. It's ugly.

To achieve sane behavior, I need to stop using subprocess.run() for any process that should not be killed with SIGKILL and replace them with direct usage of subprocess.Popen(), possibly in a with statement. It might be okay for things like astrometry.net or ntpq subprocesses to be killed that way, but it's certainly not okay for most programs in this package to be killed that way.

The offending code in Python 3.10 is this: https://github.com/python/cpython/blob/ae8a721c2ba2a48de0cb69fa868fa8fc207b129d/Lib/subprocess.py#L503-L527

The __exit__() method for the Popen class appears to have more reasonable behavior: https://github.com/python/cpython/blob/ae8a721c2ba2a48de0cb69fa868fa8fc207b129d/Lib/subprocess.py#L1037-L1063

  • Replace unsafe usage of subprocess.run() with subprocess.Popen() and Popen.communicate() or Popen.wait().