rpjohnst / dejavu

Game Maker reimplementation
https://dejavu.abubalay.com
Apache License 2.0
70 stars 7 forks source link

`vm::Thread` leaks arrays on the VM stack on errors #44

Closed rpjohnst closed 4 years ago

rpjohnst commented 4 years ago

Tangential to #40, the interpreter leaks any arrays stored in local variables when it exits with an error. Global and instance member arrays are also never freed when vm::World or an instance is dropped. Much of this will be solved along with #40 by implementing Drop for vm::Value.

However, things are further complicated by the way vm::Thread's stack of registers works- only the compiler knows what type of value each register stores, and that information is not preserved except in the way generated bytecode releases local variables on normal script returns. This makes for a simpler VM, where instructions can just assume the type of their register operands and overwrite them at will without dropping whatever was there before.

I can see two reasonable ways to address this:

Both of these tie into features that GM:S added to GML eventually, which we may thus want to add for their own sake. They also have some implementation overlap with each other.

But on the other hand this is not a big deal yet because nothing really tries to recover from script errors. I had hoped to avoid any kind of stack walking or unwinding for a while, but once that changes this will need a solution.

rpjohnst commented 4 years ago

After #45, global and instance member arrays are freed correctly. However, stack values will become a real problem once the interpreter becomes reentrant, as this will enable engine APIs to recover from errors. We won't need this capability (at first?) but it would be nice to avoid such a hidden expectation on engine APIs.

Fortunately, there may be a much simpler solution than the two proposed above, inspired by #46. There, iterator registers "borrow" from a parallel stack of RcVec<Entity> entries, which can be safely popped as the program completes each with loop.

Here, GML value registers would "borrow" from a parallel stack of vm::Value entries, which could be safely popped as the program returned from each function. This is slightly more complicated- unlike with iterators, the values themselves do not follow a strict stack discipline, nor is there even a statically-knowable number of them.

Each function call (or other program span where array values can be known to die?) would save the current stack size on entry and truncate the stack back to that size on exit. Any instruction sequence creating an array (using a call or ToArray op) would push it onto the stack, and then either leave it there (if stored in a local var) or pop it (if stored in a global or instance member).

This also has two side benefits: Immediately, it removes the need for Release ops in the function epilogue, shrinking the live ranges of all local vars! Longer term, it gives us a straightforward way to control exactly where copy-on-write array refcounts are modified.