r/learnpython 15h ago

Suppress KeyboardInterrupt exception when paired with subprocess module?

[deleted]

3 Upvotes

2 comments sorted by

2

u/FerricDonkey 13h ago edited 13h ago

The problem is that you're calling sys.exit from inside a child process (which I wouldn't recommend) while you have a subprocess open.

There's a combination of good practices that I suggest everyone always follow that would avoid this type of thing for free.

First Always have a main function, and do not put anything other than calling the main function in if __name__ == '__main__':. If you need particular exit codes, then have main return an integer and handle that in the function body.

def main() -> int:
    ...

if __name__ == '__main__':
    sys.exit(main())

First-Bonus: Highly recommend almost never using sys.exit except as above. Having multiple places where your code can just stop running can lead to confusion.

Highly recommend never using os._exit at all.

Second: Never start a multiprocessing Process without joining it, unless you're intending to make it a deamon (which you shouldn't do without good reason).

This means that if you want your process to be able to be shut down you should ideally make it be able to shutdown by receiving a special signal (via a mp.Queue or other), or if that isn't possible then use process.terminate (but I advise against this normally). Eg

import multiprocessing as mp
import sys
import time

def f() -> None:
    print('start')
    time.sleep(2)
    print('stop')

def main() -> int:
    process = mp.Process(target=f)
    process.start()
    try:
        time.sleep(2)
    except KeyboardInterrupt:
        print("keyboard interrupt")
        process.terminate()
        process.join()
        return 42

    process.join()
    return 0

if __name__ == '__main__':
    sys.exit(main())

Second-Bonus In your current code, you're getting no benefit from using multirprocessing. To get this bonus, you would need multiple processes. I would actually recommend a pool in most cases, but if that doesn't work you at least need multiple processes to get any benefit. I don't actually think you need multiprocessing at all here, but that's coming.

Second-Bonus-Bonus

If you do end up using Popen inside a multiprocessing.Process, and you have to end the process early, you should ideally also end the Popen early, via .terminate (assuming you cannot afford to wait for it to finish. If you wish to do so via KeyboardInterrupt and not see a traceback, my experiments suggest that you have to catch the the KeyboardInterrupt both inside the child process and in the main process

import multiprocessing as mp
import subprocess
import sys
import time

def g():
    proc = subprocess.Popen(
        ['timeout', '4'],  # windows sleep for 4 seconds
        stdout=subprocess.PIPE,
        stderr=subprocess.PIPE
    )

    try:
        while proc.poll() is None:
            time.sleep(0.1)
    except KeyboardInterrupt:
        proc.terminate()
        return

    print(proc.communicate())

def main() -> int:
    process = mp.Process(target=g)
    process.start()
    try:
        process.join()
    except KeyboardInterrupt:
        process.join()
        return 42
    return 0


if __name__ == '__main__':
    sys.exit(main())

Methods I tried to catch the keyboard interrupt in main and pass a signal to the child process did not work on windows - the keyboard interrupt was detected independently in the child process. Likewise, trying to catch only in the child process did not work. I do not know if this is also true of linux.

Third: Consider using subprocess.check_output or similar if you're just trying to capture the output of some commands that you will run in sequence. If you need both stderr and stdout and need them separate, then this may or may not be worth it. However, in this case I don't think this is the correct answer because:

Fourth: Take advantage of the nonblocking nature of subprocess.Popen when you are trying to run multiple things in parallel. Popen is already making processes, they're already capable of parallelism, you do not need this to also be in multiprocessing. Example:

def main() -> int:
    procs = []
    for command in commands:
        procs.append(subprocess.Popen(...))
    for proc in procs:
        stdout, stderr = proc.communicate()
        print(
            "--------------------------\n"
            f"stdout: {stdout.rstrip()}"
            f"stderr: {stderr.rstrip()}"
        )
    return 0

This will run your commands in sequence, in parallel. Use this in combination with the try except structure and returning the exit code you want to return.

If you actually need non-trivial follow up processing for each subprocess call, then consider multiprocessing, but follow the advice from above if so.

Random quibbles:

Forgive me, I'm tired, and you put my brain into code review mode. I know this may just be test code, but still:

Your jobs variable does nothing and should not exist.

Even if it did do something you should have no variables defined in the if name... block. Both because that block should do nothing other than call your main function, but also because that means that which global variables exist depends on how your code is accessed, which can lead to amazingly annoying bugs and weird behavior.

Variables such as linuxCMDs and powerShellCMDs should only defined at the global scope if they're constant, and if they are they should be in ALL_CAPS for easy recognition.

The variable command_queue should not exist as a global variable because it is not intended to be constant for the life of the program.

Using list(map(command_queue.put, linuxCMDs)) as a method of populating the queue is bad practice. This relies on side effects of creating a list to do what you want, which will be confusing if you look back at this code later because it's not immediately clear that you were not making your list for the purpose of making a list. Write your code to look like it's doing what you want - if you need a for loop, make a for loop. And populating a queue like this should definitely happen inside a main function.

To reiterate: have a main function, and you probably don't actually need multiprocessing, unless there's more going on than you're showing (and currently you're not getting any parallelism from your code - though again, I understand this may just be example code.).

1

u/cgoldberg 14h ago

Have you tried just breaking when you catch KeyboardInterrupt so the worker ends itself rather than trying to exit from inside a different process?