app: fix control-C behavior

By default, the Python interpreter will dump stack if you interrupt
the process by pressing control-C at the terminal (or otherwise send
SIGINT):

  $ python my_script.py
  ^CTraceback (most recent call last):
    <stack trace goes here>
  KeyboardInterrupt

This is true on Unix and Windows. For more details on the
interpreter's overrides to the default signal handler, see:

https://docs.python.org/3/library/signal.html#note-on-signal-handlers-and-exceptions

West catches KeyboardInterrupt because we do not want to dump stack in
this case. This is an expected exception, and west only wants to dump
stack on unexpected exceptions.

However, we shouldn't be exiting with status code 0 in this case,
because we did not successfully complete the west command.

Instead, we should use the conventional nonzero exit code that command
line programs respond with when they exit asynchronously due to
SIGINT. This code varies by platform, and following conventions
properly influences the behavior of the caller's environment to make
our exit cause clear. One important case is when west is run as part
of a shell or .bat script that checks "$?" or "%ERRORLEVEL%"
respectively to determine what happened with the west process.

On Unix, exiting with the expected status code is not enough, however.
If that's all we do, the calling shell will not detect that we were
interrupted. This results in badness in situations like this (tested
in bash on Linux):

  $ while true; do west foo ... ; done

Setting the "I was interrupted" exit code alone is not enough to
signal to the shell that the west process was interrupted: the while
loop will still continue forever, which is not what the user wants.

Instead, delegating to the platform's default SIGINT handler on Unix
properly informs the shell that the child was interrupted and control
flow should exit the loop.

Use platform-specific exception handling to fix these issues.

See comments for references to more details.

Fixes: #636
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This commit is contained in:
Martí Bolívar 2023-03-01 10:38:20 -08:00 committed by Marti Bolivar
parent 3877745955
commit 9789617cd5
1 changed files with 34 additions and 1 deletions

View File

@ -18,7 +18,9 @@ from io import StringIO
import logging import logging
import os import os
from pathlib import Path, PurePath from pathlib import Path, PurePath
import platform
import shutil import shutil
import signal
import sys import sys
from subprocess import CalledProcessError from subprocess import CalledProcessError
import tempfile import tempfile
@ -384,7 +386,38 @@ class WestApp:
else: else:
self.run_extension(args.command, argv) self.run_extension(args.command, argv)
except KeyboardInterrupt: except KeyboardInterrupt:
sys.exit(0) # Catching this avoids dumping stack.
#
# Here we replicate CPython's behavior in exit_sigint() in
# Modules/main.c (as of
# 2f62a5da949cd368a9498e6a03e700f4629fa97f), but in pure
# Python since it's not clear how or if we can call that
# directly from here.
#
# For more discussion on this behavior, see:
#
# https://bugs.python.org/issue1054041
if platform.system() == 'Windows':
# The hex number is a standard value (STATUS_CONTROL_C_EXIT):
# https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/596a1078-e883-4972-9bbc-49e60bebca55
#
# Subtracting 2**32 seems to be the convention for
# making it fit in an int32_t.
CONTROL_C_EXIT_CODE = 0xC000013A - 2**32
sys.exit(CONTROL_C_EXIT_CODE)
else:
# On Unix, just reinstate the default SIGINT handler
# and send the signal again, overriding the CPython
# KeyboardInterrupt stack dump.
#
# In addition to exiting with the correct "dying due
# to SIGINT" status code (usually 130), this signals
# to the calling environment that we were interrupted,
# so that e.g. "while true; do west ... ; done" will
# exit out of the entire while loop and not just
# the west command.
signal.signal(signal.SIGINT, signal.SIG_DFL)
os.kill(os.getpid(), signal.SIGINT)
except BrokenPipeError: except BrokenPipeError:
sys.exit(0) sys.exit(0)
except CalledProcessError as cpe: except CalledProcessError as cpe: