dylibso / chicory

Native JVM WebAssembly runtime
Apache License 2.0
437 stars 34 forks source link

WasiExitException thrown every time proc_exit is called #533

Open bhelx opened 1 day ago

bhelx commented 1 day ago

Reported in Zulip by Stefan


Hello from Go

Sept. 19, 2024 1:44:15 PM com.dylibso.chicory.log.SystemLogger log
INFORMATION: proc_exit: [0@i32]

An error occurred.
com.dylibso.chicory.runtime.exceptions.WASMMachineException: com.dylibso.chicory.wasi.WasiExitException: Process exit code: 0

It appears this exception is being thrown even when the program exits successfully: https://github.com/dylibso/chicory/blame/b69237737c6f05f74d4c3fafbb1e32a350953e9a/wasi/src/main/java/com/dylibso/chicory/wasi/WasiPreview1.java#L1043

I think there are two options to fix:

  1. only throw the exception when the status-code is non-zero
  2. don't throw the exception at all and propagate the status code through some other means

Though I think it depends on what was the intention behind the exception so perhaps @electrum can provide some insight.

andreaTP commented 1 day ago

It appears this exception is being thrown even when the program exits successfully

I'm convinced that the current implementation is about "the best" we can offer in terms of consistency. I think, it is pretty useful to be able to handle the exception "from the outside".

Regarding your options:

  1. I think that doing different things depending on the status code is more opinionated than the current approach, still you can do it by plugging in a custom implementation of the proc_exit Host Function
  2. I have no idea how to design this, also, I would not change a lot the design to support an optional, and easily overridable function

FWIW, additional context is that the only compiler(I'm aware of) that makes use of this Wasi function is the "standard" Go compiler.

bhelx commented 1 day ago

My assumption was that proc_exit is a proxy for the exit syscall, so it's not necessarily compiler specific as any c or rust program could call exit. though i may be wrong there. If the answer is to leave it the way it is, that's fine I think. maybe we just document that it may throw this exception and you need to catch it. Or put it in the example in the Readme

andreaTP commented 1 day ago

it's not necessarily compiler specific

It shouldn't, but, apparently, it's not widely used in practice. Happy to document it, should it be in a "standard Go" specific section/FAQ list/whatever?

Fun fact, looking closer at this, if you don't throw the Exception nor explicitly stop the runtime while running "standard Go" the runtime is going to fail trying to perform a "sleep"(and we don't have the wasi function implemented for it yet).

bhelx commented 1 day ago

Fun fact, looking closer at this, if you don't throw the Exception nor explicitly stop the runtime while running "standard Go" the runtime is going to fail trying to perform a "sleep"(and we don't have the wasi function implemented for it yet).

I assume this is because you actually need to explicitly stop the program, which the exception is implicitly doing. There is code after the exit but it should never be executed because calling the syscall on an OS would halt the program. FWIW it does seem that any time you call exit it's turning that into proc_exit. Verified with:

#include <stdlib.h>
#include <stdio.h>

int main() {
    printf("This program will now exit.\n");
    exit(0);
    printf("This line will not be printed.\n");
    return 0;
}
andreaTP commented 1 day ago

Thanks for looking closer at it!

According to those further findings, I think that "always throwing an exception"(current implementation) is the most idiomatic way to implement the proc_exit wasi function, as it requires the user to handle the situation accordingly.

electrum commented 23 hours ago

As you've shown, the proc_exit call must not return, so it must throw an exception. In order for Chicory to not throw an exception to the user Java code, we would need to add special handling somewhere in Chicory that knows about WASI (or is pluggable somehow), which doesn't seem reasonable.