What does this do?

Do not use terminate on Python process objects, it is not portable. The only things that are portable are:

• requesting termination through the usual communication channel
• the kill method of process objects

You should put this RPC termination request into stop_controller, to replace proc.terminate.

On the contrary: both terminate() and kill() are portable. Just on windows kill() is an alias of terminate().

It is not portable, they do different things. terminate sends a signal that can be caught on Unix and cannot on Windows.

The correct way to terminate a process in ARTIQ is request termination via IPC/RPC, wait, and kill on timeout. This is not what your code does.

On Linux, it requests termination then immediately sends a signal to the process that is in general not caught (signal implementations in Python are racy so let's not touch them) and therefore will cause ungraceful termination, waits, and kills on timeout.

On Windows, it requests termination then immediately kills without any time for the process to gracefully end. The wait and second kill are useless.

But the way to terminate a process on Unix is to SIGTERM and then SIGKILL. Exactly because with SIGTERM the process has the chance to do cleanup. This is not useless on linux and is relevant if the controller is stuck, does not react to terminate(), but the underlying driver still reacts to SIGTERM and expects to clean up. If you SIGKILL in that situation will do more harm than good. The following sequence seems to cover both linux and windows and the rpc protocol:

client.terminate()  # with a timeout
if no timeout:
  proc.wait()  # with timeout
  if no timeout:
    return
proc.terminate()
proc.wait() # with timeout
if no timeout:
  return
proc.kill()
proc.wait()  # with timeout

I would make the intent clearer by adding a test for os.name that skips the proc.terminate on Windows and perhaps a source code comment; otherwise okay, preferably this process termination method would be used consistently throughout ARTIQ.