Save/restore function's globals in exception-safe manner. #292
Conversation
This fixes #290.
|
There are still going to be issues with running native/viper code. It won't have a mechanism to catch all exceptions and restore the globals/locals. One possible fix is to have the exception handling itself save/restore globals and locals (adds 2 words to the exception buffer). Another fix is to not have globals/locals as global variables, and instead pass them around as the current context. Note that not many functions actually need them, because most of the builtins run independent of the context. Exceptions are dir, exec, eval. rt_call_function would have to take globals as a parameter and pass it to type->call. Not very pretty. |
Well, as you see, the patch deals only with bytecode functions. I don't know how exactly to deal with native/viper code, first thought is just that - they are intuitively mostly context-free.
If you think that nlr_* can be tied so deep with Py, maybe. I'm not sure about locals though. Intuitively, we need to care about globals because functions may return between modules. But apparently, one cannot split lexically-scoped functions across modules, so there's no need to switch locals because of the reason we discuss here (but there may be other reasons...)
Sooner or later someone will raise issue of support multiple VMs (that's the only (known to me) effective way to fight GIL). So, I'd be looking positive at such change, if you think it makes sense (but yes, eventually that means defining struct vm and keeping stuff there, and pass that to every API call). |
|
Native is "just" an unrolled VM, so would need to take in globals as an argument.
That is the downside, having it tied up. Note that it doesn't hurt to restore locals, since an exception should always restore to exactly the state when nlr_push was called.
It would be nice to have no global variables, ultimately even for the GC as well. But that means pretty much all functions need a state parameter. Better (?) way is to use a dedicated register to hold state, that only needs changing when context changes. There is support for this in compilers, but I don't know how to leverage it for our purposes. Note that a dedicated thread register is really just a (fast) global variable that is saved/restored when the thread switches. We can emulate this by having a single global variable holding the current context. nlr_* would save/restore this global variable, and it would just be a generic pointer. |
|
I did it differently. |
* atmel-samd: Support patching after updating ASF4. This makes it possible to automatically fix newer code. * atmel-samd: Update ASF4 to include flash APIs for SAMD51. This is the first automatic update that has caused a few deletions where code was previously copied instead of moved. This is a prerequisite for micropython#260.
This is proposed fix for #290.
Comments:
As usual, if there're concerns what's passed in regs on ARM, globals arg can be added last instead of first to bytecode exec functions.
Otherwise, it looks kinda like breaking layer separation, for example vm suddenly knows about maps. But code execution always happens within context of some environment, and managing environments apparently a task for VM (especially if we model target exception with exceptions on meta level). But rt_globals_get/set() are there exactly to abstract it away, so they can be made accept void*,and then we don't have env implementation details leak into VM.