Wasmer: Panic hooks stop working after 2nd panic

[Jake-Shadle]

Describe the bug

If you add a panic hook in rust via std::panic::set_hook to eg. call back to a host function with panic information for display on the host, it works!...except after the 2nd panic has been hit, regardless of whether the host function is invoked or not.

wasmer-runtime 0.16.0 | rustc 1.42.0 (b8cedc004 2020-03-09) | x86_64

Steps to reproduce

I forked the wasmer Rust example at https://github.com/Jake-Shadle/wasmer-rust-example, so doing
(cd wasm-sample-app && cargo build --release) && cargo run as stated in the README will compile and wasm module and the host to show the behavior described in this bug.

Expected behavior

The hook should be invoked for each panic and run whatever code is in the hook.

Hello, World!
0: Hello, World!
2: Hello, World!
HOOK!
call #0 to 'fails' correctly captured panic 'oh no, src/lib.rs:53:5'
HOOK!
call #1 to 'fails' correctly captured panic 'oh no, src/lib.rs:53:5'
HOOK!
call #2 to 'fails' correctly captured panic 'oh no, src/lib.rs:53:5'
HOOK!
call #4 to 'fails' correctly captured panic 'oh no, src/lib.rs:53:5'

Actual behavior

The hook is executed at most twice before never executing again in the same instance.

Hello, World!
0: Hello, World!
2: Hello, World!
HOOK!
call #0 to 'fails' correctly captured panic 'oh no, src/lib.rs:53:5'
HOOK!
call #1 to 'fails' correctly captured panic 'oh no, src/lib.rs:53:5'
call #2 to 'fails' failed to capture panic information
call #3 to 'fails' failed to capture panic information

Additional context

Just to be clear, I'm not 100% sure this is a bug in wasmer, but given that it works at least twice, it kind of feels like there is a runtime issue, rather than a language/compiler/issue. This same issue can be observed if you have multiple different functions that each panic of course, and you can simply reorder the functions, and the first 2 that are executed will always do the correct thing, then the others will fail.

[MarkMcCaskey]

Thanks for the report!

Yeah, that is very strange, the reason we test two panics is that we fixed a bug on the Window's exception handler and wanted to test it. If I'm remembering the code correctly, it's really surprising that the fix wouldn't work for 3 as well...

I'll look into this today and get back to you. I'll test it on multiple platforms, but just out of curiosity is this a Windows-specific bug or happening on Unix-likes for you?

[Jake-Shadle]

Yes, I did consider that Windows specific bug since my co-worker was the one who filed the issue, and this bug might indeed be related, but this is happening for me on Linux.

[MarkMcCaskey]

Hmm, so I looked into this a bit and it looks like Wasmer is correctly capturing the trap every time, you can print out the error result in Err(e) and it's always the same and doing another type of trap, like calling:

fn host_panic(ctx: &mut Ctx, ptr: WasmPtr<u8, Array>, len: u32) -> Result<(), String> {
    let memory = ctx.memory(0);
    let s = ptr.get_utf8_string(memory, len).unwrap();

    Err(format!("Trapping with {}", s))
}

works every time. So I'm inclined to believe that the third attempt to panic is triggering an abort or otherwise hitting an unreachable code on the Rust side but after looking at the documentation, I can't figure out _why_ that would be happening 🤔

[MarkMcCaskey]

Yeah, removing the Once on the guest side gives us

call #1 to 'fails' correctly captured panic 'cannot modify the panic hook from a panicking thread, src/libstd/panicking.rs:101:9' as the second error:

It seems that the way the panic is being caught is not ever marking the thread as not panicking, so it's likely triggering code that aborts if Rust panics while panicking.

[Jake-Shadle]

Hmm interesting. FWIW this is basically the same panic hooking code that https://github.com/rustwasm/console_error_panic_hook uses but there aren't any bug reports about 3+ panics not working correctly. Seems like I might need to dig into the Rust panic handling code a bit.

[syrusakbary]

This issue should be solved in the refactor.

[syrusakbary]

Closing the issue since the refactor has now landed in master