From 9789617cd5becaf2b7d75aee996e9e3477cbc4a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=AD=20Bol=C3=ADvar?= <marti.bolivar@nordicsemi.no>
Date: Wed, 1 Mar 2023 10:38:20 -0800
Subject: [PATCH] app: fix control-C behavior
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 src/west/app/main.py | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/src/west/app/main.py b/src/west/app/main.py
index fd77fb9..7bb087a 100755
--- a/src/west/app/main.py
+++ b/src/west/app/main.py
@@ -18,7 +18,9 @@ from io import StringIO
 import logging
 import os
 from pathlib import Path, PurePath
+import platform
 import shutil
+import signal
 import sys
 from subprocess import CalledProcessError
 import tempfile
@@ -384,7 +386,38 @@ class WestApp:
             else:
                 self.run_extension(args.command, argv)
         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:
             sys.exit(0)
         except CalledProcessError as cpe: