REPL: Ctrl-C doesn't raise KeyboardInterrupt

[notro]

Pressing Ctrl-C when sleeping aborts the operation but doesn't raise an exception like in CPython.

CircuitPython:

Adafruit CircuitPython patchbase-3-gedb366e7f-dirty on 2018-08-06; Adafruit Feather M0 Express with samd21g18
>>> import time
>>> time.sleep(10)
>>>

CPython:

Python 3.4.2 (default, Oct 19 2014, 13:31:11)
[GCC 4.9.1] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import time
>>> time.sleep(10)
^CTraceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyboardInterrupt
>>>

Another problem is that that the Ctrl-C event gets stuck so the next sleep is also aborted:

Adafruit CircuitPython patchbase-3-gedb366e7f-dirty on 2018-08-06; Adafruit Feather M0 Express with samd21g18
>>> import time
>>> t = time.monotonic(); time.sleep(1); print(time.monotonic() - t)
1.0
>>> t = time.monotonic(); time.sleep(1); print(time.monotonic() - t)
[[PRESS CTRL-C]]
0.333984
>>> t = time.monotonic(); time.sleep(1); print(time.monotonic() - t)
0.0
>>>

[dhalbert]

Ctrl-C raises an interrupt in 2.x and 3.0.0, so this is something that has changed in master only recently.

Adafruit CircuitPython 2.3.1 on 2018-05-07; Adafruit Metro M0 Express with samd21g18
>>> import time
>>> time.sleep(10)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyboardInterrupt: 
>>> 

Adafruit CircuitPython 3.0.0 on 2018-07-09; Adafruit Metro M0 Express with samd21g18
>>> import time
>>> time.sleep(10)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyboardInterrupt: 
>>> 

[notro]

Ok, I see that the pending exception isn't stuck either on a 3.x I have. The next time.sleep works just fine.

[notro]

This differs between 3.x and 4.x:

diff --git a/py/vm.c b/py/vm.c
index 72d0d0d60..b5f53ee9a 100644
--- a/py/vm.c
+++ b/py/vm.c
@@ -136,7 +136,7 @@ mp_vm_return_kind_t mp_execute_bytecode(mp_code_state_t *code_state, volatile mp
     #define ENTRY(op) entry_##op
     #define ENTRY_DEFAULT entry_default
 #else
-    #define DISPATCH() goto dispatch_loop
+    #define DISPATCH() break
     #define DISPATCH_WITH_PEND_EXC_CHECK() goto pending_exception_check
     #define ENTRY(op) case op
     #define ENTRY_DEFAULT default

Making that change gives the expected exception:

Adafruit CircuitPython patchbase-dirty on 2018-08-17; Adafruit Feather M0 Express with samd21g18
>>> import time
>>> time.sleep(10)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyboardInterrupt:
>>>

[tannewt]

@notro Any idea why? It looks like the change was intentional: micropython@869024d

[notro]

Hmm, 2.5% perfomance improvement is nice.

Here are the various places where mp_pending_exception is checked:

py/vm.c:

mp_vm_return_kind_t mp_execute_bytecode(mp_code_state_t *code_state, volatile mp_obj_t inject_exc) {
...
pending_exception_check:
                MICROPY_VM_HOOK_LOOP

                #if MICROPY_ENABLE_SCHEDULER
...
                #else
                // This is an inlined variant of mp_handle_pending
                if (MP_STATE_VM(mp_pending_exception) != MP_OBJ_NULL) {
                    MARK_EXC_IP_SELECTIVE();
                    mp_obj_t obj = MP_STATE_VM(mp_pending_exception);
                    MP_STATE_VM(mp_pending_exception) = MP_OBJ_NULL;
                    RAISE(obj);
                }
                #endif

ports/unix/unix_mphal.c:

STATIC void sighandler(int signum) {
    if (signum == SIGINT) {
        #if MICROPY_ASYNC_KBD_INTR
        mp_obj_exception_clear_traceback(MP_OBJ_FROM_PTR(&MP_STATE_VM(mp_kbd_exception)));
        sigset_t mask;
        sigemptyset(&mask);
        // On entry to handler, its signal is blocked, and unblocked on
        // normal exit. As we instead perform longjmp, unblock it manually.
        sigprocmask(SIG_SETMASK, &mask, NULL);
        nlr_raise(MP_OBJ_FROM_PTR(&MP_STATE_VM(mp_kbd_exception)));
        #else
        if (MP_STATE_VM(mp_pending_exception) == MP_OBJ_FROM_PTR(&MP_STATE_VM(mp_kbd_exception))) {
            // this is the second time we are called, so die straight away
            exit(1);
        }
        mp_obj_exception_clear_traceback(MP_OBJ_FROM_PTR(&MP_STATE_VM(mp_kbd_exception)));
        MP_STATE_VM(mp_pending_exception) = MP_OBJ_FROM_PTR(&MP_STATE_VM(mp_kbd_exception));
        #endif
    }
}

ports/unix/main.c:

STATIC int execute_from_lexer(int source_kind, const void *source, mp_parse_input_kind_t input_kind, bool is_repl) {
    mp_hal_set_interrupt_char(CHAR_CTRL_C);

    nlr_buf_t nlr;
    if (nlr_push(&nlr) == 0) {
...
        mp_parse_tree_t parse_tree = mp_parse(lex, input_kind);
...
        mp_obj_t module_fun = mp_compile(&parse_tree, source_name, emit_opt, is_repl);

        if (!compile_only) {
            // execute it
            mp_call_function_0(module_fun);
            // check for pending exception
            if (MP_STATE_VM(mp_pending_exception) != MP_OBJ_NULL) {
                mp_obj_t obj = MP_STATE_VM(mp_pending_exception);
                MP_STATE_VM(mp_pending_exception) = MP_OBJ_NULL;
                nlr_raise(obj);
            }
        }

        mp_hal_set_interrupt_char(-1);
        nlr_pop();
        return 0;

    } else {
        // uncaught exception
        mp_hal_set_interrupt_char(-1);
        return handle_uncaught_exception(nlr.ret_val);
    }
}

ports/unix/file.c:

STATIC mp_uint_t fdfile_write(mp_obj_t o_in, const void *buf, mp_uint_t size, int *errcode) {
...
    mp_int_t r = write(o->fd, buf, size);
    while (r == -1 && errno == EINTR) {
        if (MP_STATE_VM(mp_pending_exception) != MP_OBJ_NULL) {
            mp_obj_t obj = MP_STATE_VM(mp_pending_exception);
            MP_STATE_VM(mp_pending_exception) = MP_OBJ_NULL;
            nlr_raise(obj);
        }
        r = write(o->fd, buf, size);
    }

ports/windows/windows_mphal.c:

// Handler to be installed by SetConsoleCtrlHandler, currently used only to handle Ctrl-C.
// This handler has to be installed just once (this has to be done elswhere in init code).
// Previous versions of the mp_hal code would install a handler whenever Ctrl-C input is
// allowed and remove the handler again when it is not. That is not necessary though (1),
// and it might introduce problems (2) because console notifications are delivered to the
// application in a separate thread.
// (1) mp_hal_set_interrupt_char effectively enables/disables processing of Ctrl-C via the
// ENABLE_PROCESSED_INPUT flag so in raw mode console_sighandler won't be called.
// (2) if mp_hal_set_interrupt_char would remove the handler while Ctrl-C was issued earlier,
// the thread created for handling it might not be running yet so we'd miss the notification.
BOOL WINAPI console_sighandler(DWORD evt) {
    if (evt == CTRL_C_EVENT) {
        if (MP_STATE_VM(mp_pending_exception) == MP_OBJ_FROM_PTR(&MP_STATE_VM(mp_kbd_exception))) {
            // this is the second time we are called, so die straight away
            exit(1);
        }
        mp_obj_exception_clear_traceback(MP_OBJ_FROM_PTR(&MP_STATE_VM(mp_kbd_exception)));
        MP_STATE_VM(mp_pending_exception) = MP_OBJ_FROM_PTR(&MP_STATE_VM(mp_kbd_exception));
        return TRUE;
    }
    return FALSE;
}

ports/stm32/pendsv.c:

// This variable is used to save the exception object between a ctrl-C and the
// PENDSV call that actually raises the exception.  It must be non-static
// otherwise gcc-5 optimises it away.  It can point to the heap but is not
// traced by GC.  This is okay because we only ever set it to
// mp_kbd_exception which is in the root-pointer set.
void *pendsv_object;

void pendsv_init(void) {
    // set PendSV interrupt at lowest priority
    HAL_NVIC_SetPriority(PendSV_IRQn, IRQ_PRI_PENDSV, IRQ_SUBPRI_PENDSV);
}

// Call this function to raise a pending exception during an interrupt.
// It will first try to raise the exception "softly" by setting the
// mp_pending_exception variable and hoping that the VM will notice it.
// If this function is called a second time (ie with the mp_pending_exception
// variable already set) then it will force the exception by using the hardware
// PENDSV feature.  This will wait until all interrupts are finished then raise
// the given exception object using nlr_jump in the context of the top-level
// thread.
void pendsv_kbd_intr(void) {
    if (MP_STATE_VM(mp_pending_exception) == MP_OBJ_NULL) {
        mp_keyboard_interrupt();
    } else {
        MP_STATE_VM(mp_pending_exception) = MP_OBJ_NULL;
        pendsv_object = &MP_STATE_VM(mp_kbd_exception);
        SCB->ICSR = SCB_ICSR_PENDSVSET_Msk;
    }
}

ports/esp8266/modnetwork.c:

STATIC mp_obj_t esp_scan(mp_obj_t self_in) {
...
    wifi_station_scan(NULL, (scan_done_cb_t)esp_scan_cb);
    while (esp_scan_list != NULL) {
        // our esp_scan_cb is called via ets_loop_iter so it's safe to set the
        // esp_scan_list variable to NULL without disabling interrupts
        if (MP_STATE_VM(mp_pending_exception) != NULL) {
            esp_scan_list = NULL;
            mp_obj_t obj = MP_STATE_VM(mp_pending_exception);
            MP_STATE_VM(mp_pending_exception) = MP_OBJ_NULL;
            nlr_raise(obj);
        }
        ets_loop_iter();
    }

This is another way for us to solve this, haven't looked at performance though:

diff --git a/ports/atmel-samd/background.c b/ports/atmel-samd/background.c
index 62d891b15..b50a62ba0 100644
--- a/ports/atmel-samd/background.c
+++ b/ports/atmel-samd/background.c
@@ -30,6 +30,8 @@
 #include "usb.h"
 #include "usb_mass_storage.h"

+#include "py/mpstate.h"
+
 volatile uint64_t last_finished_tick = 0;

 void run_background_tasks(void) {
@@ -38,6 +40,13 @@ void run_background_tasks(void) {
     #endif
     usb_msc_background();
     usb_cdc_background();
+
+    if (MP_STATE_VM(mp_pending_exception) != MP_OBJ_NULL) {
+        mp_obj_t obj = MP_STATE_VM(mp_pending_exception);
+        MP_STATE_VM(mp_pending_exception) = MP_OBJ_NULL;
+        nlr_raise(obj);
+    }
+
     last_finished_tick = ticks_ms;
 }

It works, but the traceback is gone:

Adafruit CircuitPython 4.0.0-alpha-950-g1a6f6ff8e-dirty on 2018-08-18; Adafruit Feather M0 Express with samd21g18
>>>
>>> import time
>>> time.sleep(10)
KeyboardInterrupt:
>>> time.sleep(10)
KeyboardInterrupt:
>>>

[tannewt]

I'm not concerned about a bit of performance loss because its not a core focus for us. I'm also not worried about losing the traceback since its arbitrary. So, its up to you how you want to solve it. :-) Either way works for me.