py/objexcept: Add uncatchable SystemAbort exception. #9949
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9949 +/- ##
==========================================
- Coverage 98.33% 98.33% -0.01%
==========================================
Files 156 156
Lines 20520 20519 -1
==========================================
- Hits 20179 20178 -1
Misses 341 341
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
Thanks @laurensvalk Implementation-wise this makes sense to me to solve the scenario you described at the meetup, but I wondered whether we maybe need to consider cases where Python code is protected by a NLR scope that isn't managed by the VM. One example would be when the scheduler invokes a function, via But, if we make |
|
Thanks Jim.
Indeed, we currently only use it on the main thread. For full context, we'd use it like this: // callback for when stop button is pressed (force_stop=false),
// or when shutdown is requested by firmware (force_stop=true)
void pbsys_main_stop_program(bool force_stop) {
static const mp_obj_tuple_t args = {
.base = { .type = &mp_type_tuple },
.len = 1,
.items = { MP_ROM_QSTR(MP_QSTR_stop_space_button_space_pressed) },
};
static mp_obj_exception_t system_exit;
// Schedule SystemExit exception.
system_exit.base.type = force_stop ? &mp_type_SystemAbort : &mp_type_SystemExit;
system_exit.traceback_alloc = 0;
system_exit.traceback_len = 0;
system_exit.traceback_data = NULL;
system_exit.args = (mp_obj_tuple_t *)&args;
MP_STATE_MAIN_THREAD(mp_pending_exception) = MP_OBJ_FROM_PTR(&system_exit);
#if MICROPY_ENABLE_SCHEDULER
if (MP_STATE_VM(sched_state) == MP_SCHED_IDLE) {
MP_STATE_VM(sched_state) = MP_SCHED_PENDING;
}
#endif
}This code is mostly in use already, but in today's release we only have |
|
This looks very clean! I thought it would be more difficult/intrusive. If we accept this, it would need to be guarded by a config option, eg
In the case of Python IRQ handlers (eg scheduled functions) I don't think there is any problem. Because already a |
Yes, this makes sense under the assumption that |
|
Perhaps we could have mp_sched_system_abort()To encourage usage just like E.g. // Exception that can't be raised or caught by user code. Do not raise directly but
// use mp_sched_system_abort() to schedule it on the main thread.
MP_DEFINE_EXCEPTION(SystemAbort, BaseException) |
Yes that would be nicer. But one issue with that there can only be one pending exception, so a |
Thanks for looking at this. I'll update the PR.
That is a good point, and it is something we're not currently handling above (thanks!). But thinking about it now, it might even be beneficial for this use case. The firmware event loop could call |
07802c2 to
c8072e4
Compare
This adds an exception type that forces MicroPython to exit even if the user MicroPython script catches all exceptions. This exception can be raised by C code to force MicroPython to exit for safety reasons, such as very low battery or very high temperatures.
This implements mp_sched_system_exit_or_abort(), which can be used to schedule a SystemExit or SystemAbort on the main thread. This can be used by async sources to exit MicroPython if requested by a firmware process. Use abort=true if the MicroPython script must not be able to catch it, not even with a bare except. The implementation is similar to mp_sched_keyboard_interrupt(). Signed-off-by: Laurens Valk <laurens@pybricks.com>
c8072e4 to
ddae9a3
Compare
I tried to follow the pattern of |
|
See alternative in #10241. |
|
@jimmo and I had a long discussion about this today. One thing that we think is not handled by the approach here is that it cannot abort the case where a user (accidentally) does a Also note that If we are going to implement a "system abort" mechanism, I think it should be as robust as possible, ie able to abort (almost) all scenarios. |
I completely agree. 👍 Don't worry about my PR. I suppose it's served its purpose by putting it on your radar 😄 |
Yes it lead to a good discussion. The approach here can be made to work (eg |
|
Closing in favour of #10241. |
…-picos3 Waveshare pico S3: change the color order of the RBG LED
This adds an exception type that forces MicroPython to exit even if the user MicroPython script catches all exceptions.
This exception can be raised by C code to force MicroPython to exit for safety reasons, such as very low battery or very high temperatures.
I did not add it to the builtins dictionary to avoid adding new non-Python features to the language so it's just for internal use. Should there be a feature guard to avoid increasing codesize?
This is my first time looking around in
vm.cso I'm not quite sure this is the right way to do it. Feel free to consider this as a draft for alternative implementations.Context
This issue was briefly discussed in the most recent MicroPython meetup. Damien's comments are at 26:00 onwards.