runtime/abort: Add support for vm_abort in standard runtime #14419

RealJohnSmith · 2024-05-03T07:14:11Z

Add abort setup code (nlr_set_abort) to standard runtime. This makes the standard runtime respond to abort signal without any further modifications.

When aborted, the program exits with 255 exit code, to differentiate from a normal shutdown.

codecov · 2024-05-03T07:24:43Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.39%. Comparing base (64f28dc) to head (5e41c7e).
Report is 33 commits behind head on master.

❗ Current head 5e41c7e differs from pull request most recent head b1cca67

Please upload reports for the commit b1cca67 to get more accurate results.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #14419   +/-   ##
=======================================
  Coverage   98.39%   98.39%           
=======================================
  Files         161      161           
  Lines       21204    21204           
=======================================
  Hits        20864    20864           
  Misses        340      340

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-05-03T07:27:35Z

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:  +168 +0.090% 
   unix x64:    +0 +0.000% standard
      stm32:   +92 +0.024% PYBV10[incl +8(bss)]
     mimxrt:   +88 +0.024% TEENSY40
        rp2:   +88 +0.026% RPI_PICO[incl +8(bss)]
       samd:   +84 +0.032% ADAFRUIT_ITSYBITSY_M4_EXPRESS[incl +8(bss)]

RealJohnSmith · 2024-05-03T08:40:57Z

I see that some checks/ports are failing because the function nlr_set_abort is not defined. However, it is guarded by #ifdef MICROPY_ENABLE_VM_ABORT . I am not sure what the problem is. Can you please help me how to fix it?

shared/runtime/pyexec.c

dlech · 2024-05-03T14:13:02Z

shared/runtime/pyexec.c

-        if (mp_obj_is_subclass_fast(MP_OBJ_FROM_PTR(((mp_obj_base_t *)nlr.ret_val)->type), MP_OBJ_FROM_PTR(&mp_type_SystemExit))) {
+        #ifdef MICROPY_ENABLE_VM_ABORT
+        if (nlr.ret_val == NULL) { // abort
+            ret = 255;


Probably want to define a macro for this value and document it.

Unhanded exceptions still return ret = 0 though, so why is this different?

And this should probably be followed by a goto that jumps to a label just before MICROPY_BOARD_AFTER_PYTHON_EXEC() at the end of the function so that no other MicroPython runtime stuff runs.

Probably want to define a macro for this value and document it.

Ok. Where is a good place to define it?

Unhanded exceptions still return ret = 0 though, so why is this different?

Is this on purpose? I consider it a bug. In my port/usecase I want the exceptions to throw a non-success exit code and I was planning opening an additional PR later. As well as exposing the exit code from system exit, so os.exit(1) in python actually exits with the exit code according to the argument.

And this should probably be followed by a goto that jumps to a label just before MICROPY_BOARD_AFTER_PYTHON_EXEC() at the end of the function so that no other MicroPython runtime stuff runs.

Is it wrong to run the repl stuff? And the mp_hal_stdout_tx_strn function. Those are the only two things that are potentially run

My use case is this:

I am building a device which is programmable by micro python by the end users. The device/firmware provides an additional "OS" layer - display/UI/menu/settings/file system/... and API for the user program. When the user program is being executed, it is rendered on the display, sending stdout to the display and other info. When the program exits, I want to be able to show a screen "Finished successfully" or "Crashed" (triggered by an unhandled exception or os.exit(non-zero) or "Killed/aborted". The abort is triggered from the device itself/button combination. I need the abort to be instant (as perceived by the user). Which means, there is some time to finish some instructions, flush std out, ...; However no more python instructions should be run (so the program wont be able to "prevent" being killed)

For this, it makes sense to me to return exit codes on the exit. Not just always zero. I am not sure if there is a use case, where you would want to have always zero exit code. If the host fw/vm doesn't care about it, it can just ignore it. Right?

Am I missing something?

I am building a device which is programmable by micro python by the end users.

We did something similar with Pybricks. What we ended up doing was just raise SystemExit to stop user programs. We only use the abort if we are powering off the device. (If a user manages to somehow always catch the SystemExit, they will have to power off the device, but this doesn't really seem to happen that we've notified).

Here is the source if you are interested: https://github.com/pybricks/pybricks-micropython/blob/79a0e010ac36fb64593d5ef5b213b64f1a03985b/bricks/_common/micropython.c#L229

If the host fw/vm doesn't care about it, it can just ignore it. Right?

Makes sense to me.

I see. Should one be preferred over the other? Or is this approach OK as well?
Is there anything blocking this merge at the moment?

I think we will have to wait for @dpgeorge to chime in here (sometimes it can take a while before he has time).

shared/runtime/pyexec.c

            // at the moment, the value of SystemExit is unused
            ret = pyexec_system_exit;
-        } else {
+        } else { // other exception
            mp_obj_print_exception(&mp_plat_print, MP_OBJ_FROM_PTR(nlr.ret_val));
            ret = 0;
        }


shared/runtime/pyexec.c

Signed-off-by: John Smith <jsmith@jsmith.cz>

RealJohnSmith · 2024-05-17T12:22:06Z

Ok, in the meantime, I have implemented the functionality, which sets exit code according to the SystemExit exception. It is set up such, that it is reverse compatible (the current exit codes won't change for anyone, unless they decide to use this by setting the appropriate exit code values)

Signed-off-by: John Smith <jsmith@jsmith.cz>

RealJohnSmith · 2024-05-17T13:00:16Z

There is a unit test failing, something with thread locking, but I am not sure what the output of the test means or how is it related to my code. Can you please advice/help? Thanks

dlech · 2024-05-17T13:14:16Z

That is a flaky test that fails randomly, so nothing to worry about.

RealJohnSmith · 2024-05-17T13:32:16Z

Ok, thanks

RealJohnSmith · 2024-05-20T07:32:44Z

What is the current status? All is ready and we are waiting for @dpgeorge? Is there something that I can do in the meantime, to prepare or fix something, to speed things up?

RealJohnSmith force-pushed the enable-abort-support branch from acf431b to 2e3e0c9 Compare May 3, 2024 08:33

dlech reviewed May 3, 2024

View reviewed changes

RealJohnSmith force-pushed the enable-abort-support branch 2 times, most recently from f3015ad to aaa8d3e Compare May 6, 2024 10:20

dlech reviewed May 6, 2024

View reviewed changes

shared/runtime/pyexec.c Outdated Show resolved Hide resolved

RealJohnSmith force-pushed the enable-abort-support branch 2 times, most recently from deb4b0e to 5fac003 Compare May 10, 2024 10:56

runtime/abort: Add support for vm_abort in standard runtime.

efe6c3e

Signed-off-by: John Smith <jsmith@jsmith.cz>

RealJohnSmith force-pushed the enable-abort-support branch from 5fac003 to efe6c3e Compare May 10, 2024 11:01

RealJohnSmith force-pushed the enable-abort-support branch 2 times, most recently from a977bfb to 403f9b2 Compare May 17, 2024 12:31

runtime/exitcodes: Set exit code according to the SystemExit exception.

b1cca67

Signed-off-by: John Smith <jsmith@jsmith.cz>

RealJohnSmith force-pushed the enable-abort-support branch from 403f9b2 to b1cca67 Compare May 17, 2024 12:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime/abort: Add support for vm_abort in standard runtime #14419

runtime/abort: Add support for vm_abort in standard runtime #14419

RealJohnSmith commented May 3, 2024

codecov bot commented May 3, 2024 •

edited

github-actions bot commented May 3, 2024 •

edited

RealJohnSmith commented May 3, 2024

dlech May 3, 2024

dlech May 3, 2024

RealJohnSmith May 6, 2024

RealJohnSmith May 6, 2024

RealJohnSmith May 6, 2024

RealJohnSmith May 10, 2024

RealJohnSmith May 10, 2024

dlech May 10, 2024

RealJohnSmith May 13, 2024

dlech May 13, 2024

This comment was marked as outdated.

RealJohnSmith commented May 17, 2024

RealJohnSmith commented May 17, 2024

dlech commented May 17, 2024

RealJohnSmith commented May 17, 2024

RealJohnSmith commented May 20, 2024

runtime/abort: Add support for vm_abort in standard runtime #14419

Are you sure you want to change the base?

runtime/abort: Add support for vm_abort in standard runtime #14419

Conversation

RealJohnSmith commented May 3, 2024

codecov bot commented May 3, 2024 • edited

Codecov Report

github-actions bot commented May 3, 2024 • edited

RealJohnSmith commented May 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as outdated.

RealJohnSmith commented May 17, 2024

RealJohnSmith commented May 17, 2024

dlech commented May 17, 2024

RealJohnSmith commented May 17, 2024

RealJohnSmith commented May 20, 2024

codecov bot commented May 3, 2024 •

edited

github-actions bot commented May 3, 2024 •

edited