What if this raises an exception?

Can you update the comment that this is LIFO ?

In CPython a SystemExit would still cause an actual exit, is it worth adding that?

These are currently only being run during an exit / soft-restart so I wouldn't think the handling should change?
Though maybe the exception shouldn't be printed out if it's a SystemExit

so I wouldn't think the handling should change?

It is actually changing: for the unix port an exception called during the atexit callback would result in exit code 1 (because of an 'FATAL: uncaught NLR', maybe not ideal, but at least it's a non-zero exit code and that is what matters). But with this implementation the exception would be swallowed (and printed). That should not change imo and this should remain CPython-compatible in the sense that an exception during atexit is treated as an exception. Note CPython does this:

If an exception is raised during execution of the exit handlers, a traceback is printed (unless SystemExit is raised) and the exception information is saved. After all exit handlers have had a chance to run, the last exception to be raised is re-raised.

But given your question, for other ports I assume raising an exception here would actually change behavior?

Though maybe the exception shouldn't be printed out if it's a SystemExit

That's indeed what CPython does.

Ah, that detail helps thanks! The filter on whether to print the exception is easy enough to implement certainly. Maybe then just overriding the exit code in the exception handler here is enough to broadly match cpython, as opposed to keeping a copy of the exception details to pass them onto any default handler.

Maybe then just overriding the exit code in the exception handler here is enough to broadly match cpython

Probably, but in the end it would be nicer to fix this properly:

as opposed to keeping a copy of the exception details to pass them onto any default handler.

Ideally the only thing which would be needed is to store nlr.ret_val then if not NULL raise it again after having called all handlers. Should not be an issue code-size-wise. Passing it onto the default handler would then happen automatically because the main code knows how to do that; however, this requires that mp_sys_atexit_execute is being called within execute_from_lexer (or whatever the counterpart is named for the various ports).

This is not the case for the unix port now, nor for the other ports. I don't know why though, and all in all this is maybe not very logical/expected; e.g. for the stm32 port it runs after deinitializing pretty much all of the hardware: is there a specific reason you did it that way? Would it not be more sound if there's not many restrictions to what can run in the atexit handler i.e. can still use hardware if it chooses to etc?

Also if this atexit handler runs within the normal execution phase, you can probably use an normal mp_obj_list_t or similar instead of manually implementing a linked list, which should also reduce code size?

This comment was added in 310b3d1

Does anyone know why this would be the case and if it's still relevant?

CPython doesn't seem to update the process exit code based on anything which happens in an atexit handler (I checked by first reading the source and then writing a silly example):

import atexit

def ohno():
    raise SystemExit(99)

atexit.register(ohno)

raise RuntimeError("fail")

❯ python atexit.py
Traceback (most recent call last):
  File "/home/gus/ry/george/tmp/atexit.py", line 8, in <module>
    raise RuntimeError("fail")
RuntimeError: fail
Exception ignored in atexit callback <function ohno at 0x7e1cfef93600>:
Traceback (most recent call last):
  File "/home/gus/ry/george/tmp/atexit.py", line 4, in ohno
    raise SystemExit(99)
SystemExit: 99
❯ echo $status
1

(Similarly if you comment the raise RuntimeError() line, the exit code becomes 0.)

Removing some of the machinery for this might(?) save a small amount of code size on bare metal ports as well, if we're lucky...?

That's an interesting point, yeah I assumed we should capture any failures here but for code size and compatibility it's worth trying without

I haven't tested this, but placing this here in parse_compile_execute() seems like it might have some unexpected consequences? For example, if you put an atexit.register() call in boot.py then will the handler be run after executing boot.py and before executing main.py?

(I think this draws attention to a shortcoming in the current architecture, which is that AFAIK there's no non-hardware-specific soft reset hook function that all ports can call for things which need cleanup in the soft reset path.)

Yes I've realised similar issues with this location and been testing / looking at other ways to add this cleanly.

I've actually been fighting the urge to undertake a broad refactor & consolidation of soft-reset / shutdown handling in ports to create the "correct" place to run the handler code! I'd rather not block this change behind such a large refactor though.

Perhaps for now I should go back to my earlier plan to just add the execute call to an appropriate place in each main.c
Then they'll all get cleaned up anyway if I get to the refactor.

Perhaps for now I should go back to my earlier plan to just add the execute call to an appropriate place in each main.c Then they'll all get cleaned up anyway if I get to the refactor.

That sounds like a good approach to me. FWIW, I have also felt this urge... but better to do it as a pure refactor, for sure.