Add disable_signals feature for debugging #300
Conversation
Okay thanks. I hadn't really thought about making this robust for multi-processing but it makes sense that we would want that for the things you're working on. I'll update and test this to include that. |
I'm unsure what needs to be done for the lind_thread_exit parts. I think we only need to disable where it sets the underlying epoch? I've fixed the other pieces but I'm trying to figure out how to pass down a feature during compilation into a subcrate to test. If anyone has experience with that let me know! |
|
@Yaxuan-w I've confirmed this now works for multi-process programs with asyncify (but no epoch insertion) and now can compile conditionally with subcrate passthrough. Can you check this out again? Thanks! |
|
If we don't disable If we don't disable signal in multi-process part, it will also panic: I tested with lind@0fe82b11ea86:~/lind-wasm$ src/wasmtime/target/debug/wasmtime run --allow-precompiled --wasi threads=y --wasi preview2=n tests/unit-tests/process_tests/deterministic/thread.cwasm
Hello from thread
thread 'main' panicked at crates/rawposix/src/interface/signal.rs:66:14:
null pointer dereference occurred
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread caused non-unwinding panic. aborting.
Aborted (core dumped)Also lind@0fe82b11ea86:~/lind-wasm$ src/wasmtime/target/debug/wasmtime run --allow-precompiled --wasi threads=y --wasi preview2=n tests/unit-tests/process_tests/deterministic/forkdup.cwasm
I'm the child
back to stdout
thread 'lind-fork-2' panicked at crates/rawposix/src/interface/signal.rs:27:9:
null pointer dereference occurred
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread caused non-unwinding panic. aborting.
Aborted (core dumped)Besides those, could we also document command lines to use this feature at somewhere? |
|
@Yaxuan-w I think my last commits fixes the bug you mentioned. I'm going to look into the documentation but as discussed today we may want to do some general modifications. |
|
@qianxichen233 can you review this? |
src/RawPOSIX/src/interface/signal.rs
Outdated
| unsafe { *epoch } | ||
| #[cfg(feature = "disable_signals")] | ||
| { | ||
| return 1; // if were disabling signals this function is unescessary so we avoid a potential null ptr dereference |
There was a problem hiding this comment.
This seems like a very rough and unstable solution for me. A better way to handle this might be to wrap all the places where get_epoch_state is called within a cfg block as well.
There was a problem hiding this comment.
Okay I updated it, does this seem better?
This PR adds a compile feature to disable the signal epoch checks for debugging programs without signals. This helps for debugging and performance comparisons.