Add an exception handler on Windows #14582
Conversation
|
I tested this by forcing a segfault: diff --git a/crates/uv-client/src/base_client.rs b/crates/uv-client/src/base_client.rs
index 9ddc30e75..45fe0e4e5 100644
--- a/crates/uv-client/src/base_client.rs
+++ b/crates/uv-client/src/base_client.rs
@@ -266,6 +266,7 @@ impl<'a> BaseClientBuilder<'a> {
}
pub fn build(&self) -> BaseClient {
+ unsafe {(1 as *mut i32).write(1);}
// Create user agent.
let mut user_agent_string = format!("uv/{}", version());
Without this patch, it exits quietly: With the patch I get a relatively short error, and Very much open to tweaking the output format. |
geofft
force-pushed
the
windows-exception-handler
branch
from
4f98bf7 to
7f978f3
Compare
| } | ||
| eprintln!("stack backtrace:\n{backtrace:#}"); | ||
| } | ||
| EXCEPTION_CONTINUE_SEARCH |
There was a problem hiding this comment.
Is there a reason we'd prefer EXCEPTION_CONTINUE_SEARCH (0) over EXCEPTION_EXECUTE_HANDLER (1)?
There was a problem hiding this comment.
I don't totally understand the difference in behavior here when we're talking about an unhandled exception filter of which there is only one (as opposed to a vectored exception handler, SEH, etc. where there might actually be a next thing to call). My vague sense is that this alerts a debugger (as a "second chance exception") instead of crashing the process, which seems good?
(Some things I am reading on the internet, e.g., this, and the relevant WINE source, make me think that under the hood the only thing that exists is SEH / unwinding, and something very low level sets up a SEH catch block around main that calls some global function pointer and all that SetUnhandledExceptionFilter does is update that pointer.)
In practice I did this because Google Crashpad returns EXCEPTION_CONTINUE_SEARCH, not EXCEPTION_EXECUTE_HANDLER.
There was a problem hiding this comment.
👍 Makes sense if the goal is to delegate to a possible outer SE handler (if any exists) and not treat this filter as the final handler. If we wanted to do filter chaining we'd need to keep the prev filter reference alive and loaded so this is probably good enough.
We've seen a few cases of uv.exe exiting with an exception code as its exit status and no user-visible output (astral-sh#14563 in the field, and astral-sh#13812 in CI). It seems that recent versions of Windows no longer show dialog boxes on access violations (what UNIX calls segfaults) or similar errors. Something is probably sent to Windows Error Reporting, and we can maybe sign up to get the crashes from Microsoft, but the user experience of seeing uv exit with no output is poor, both for end users and during development. While it's possible to opt out of this behavior or set up a debugger, this isn't the default configuration. (See https://superuser.com/q/1246626 for some pointers.) In order to get some output on a crash, we need to install our own default handler for unhandled exceptions (or call all our code inside a Structured Exception Handling __try/__catch block, which is complicated on Rust). This is the moral equivalent of a segfault handler on Windows; the kernel creates a new stack frame and passes arguments to it with some processor state. This commit adds a relatively simple exception handler that leans on Rust's own backtrace implementation and also displays some minimal information from the exception itself. This should be enough info to communicate that something went wrong and let us collect enough information to attempt to debug. There are also a handful of (non-Rust) open-source libraries for this like Breakpad and Crashpad (both from Google) and crashrpt. The approach here, of using SetUnhandledExceptionFilter, seems to be the standard one taken by other such libraries. Crashpad also seems to try to use a newer mechanism for an out-of-tree DLL to report the crash: https://issues.chromium.org/issues/42310037 If we have serious problems with memory corruption, it might be worth adopting some third-party library that has already implemented this approach. (In general, the docs of other crash reporting libraries are worth skimming to understand how these things ought to work.) Co-authored-by: samypr100 <[email protected]>
There was a problem hiding this comment.
Here's something a little interesting: on i686, we don't get a proper backtrace. I wonder if the exception handler is called on a different stack or something.
C:\Users\Administrator\uv>cargo build --target i686-pc-windows-msvc -p uv
C:\Users\Administrator\uv>target\i686-pc-windows-msvc\debug\uv run python
error: unhandled exception in uv, please report a bug:
code 0xC0000005 at address 0x2e119b7
EXCEPTION_ACCESS_VIOLATION writing 0x1
eax=00000001 ebx=0effc208 ecx=0ef48ab0 edx=07030fb0 esi=0ef44a80 edi=0effc0a4
eip=02e119b7 ebp=0ef44ec8 esp=0ef44a80 eflags=00010202
stack backtrace:
0: 0x41e265e - std::backtrace::Backtrace::capture
at /rustc/6b00bc3880198600130e1cf62b8f8a93494488cc/library\std\src\backtrace.rs:296
1: 0x1a052e1 - uv::windows_exception::unhandled_exception_filter
at C:\Users\Administrator\uv\crates\uv\src\windows_exception.rs:112
2: 0x75a354b2 - UnhandledExceptionFilter
3: 0x779c7fdf - RtlGetFullPathName_UEx
4: 0x779c7f1b - RtlGetFullPathName_UEx
| } | ||
| eprintln!("stack backtrace:\n{backtrace:#}"); | ||
| } | ||
| EXCEPTION_CONTINUE_SEARCH |
There was a problem hiding this comment.
I don't totally understand the difference in behavior here when we're talking about an unhandled exception filter of which there is only one (as opposed to a vectored exception handler, SEH, etc. where there might actually be a next thing to call). My vague sense is that this alerts a debugger (as a "second chance exception") instead of crashing the process, which seems good?
(Some things I am reading on the internet, e.g., this, and the relevant WINE source, make me think that under the hood the only thing that exists is SEH / unwinding, and something very low level sets up a SEH catch block around main that calls some global function pointer and all that SetUnhandledExceptionFilter does is update that pointer.)
In practice I did this because Google Crashpad returns EXCEPTION_CONTINUE_SEARCH, not EXCEPTION_EXECUTE_HANDLER.
geofft
force-pushed
the
windows-exception-handler
branch
from
7f978f3 to
7aa37d4
Compare
We've seen a few cases of uv.exe exiting with an exception code as its exit status and no user-visible output (#14563 in the field, and #13812 in CI). It seems that recent versions of Windows no longer show dialog boxes on access violations (what UNIX calls segfaults) or similar errors. Something is probably sent to Windows Error Reporting, and we can maybe sign up to get the crashes from Microsoft, but the user experience of seeing uv exit with no output is poor, both for end users and during development. While it's possible to opt out of this behavior or set up a debugger, this isn't the default configuration. (See https://superuser.com/q/1246626 for some pointers.)
In order to get some output on a crash, we need to install our own default handler for unhandled exceptions (or call all our code inside a Structured Exception Handling __try/__catch block, which is complicated on Rust). This is the moral equivalent of a segfault handler on Windows; the kernel creates a new stack frame and passes arguments to it with some processor state.
This commit adds a relatively simple exception handler that leans on Rust's own backtrace implementation and also displays some minimal information from the exception itself. This should be enough info to communicate that something went wrong and let us collect enough information to attempt to debug. There are also a handful of (non-Rust) open-source libraries for this like Breakpad and Crashpad (both from Google) and crashrpt.
The approach here, of using SetUnhandledExceptionFilter, seems to be the standard one taken by other such libraries. Crashpad also seems to try to use a newer mechanism for an out-of-tree DLL to report the crash: https://issues.chromium.org/issues/42310037
If we have serious problems with memory corruption, it might be worth adopting some third-party library that has already implemented this approach. (In general, the docs of other crash reporting libraries are worth skimming to understand how these things ought to work.)