Fix foreign exception handling on Windows #113323
Conversation
There is a problem in a special case with the new exception handling: * A managed code calls a 3rd party native C++ code * then a C++ exception is thrown in that code * then it is propagated through managed code * then it is caught in a 3rd party native C++ code * then a managed callback is called from the catch handler * then the exception is rethrown using C++ "throw" * then it propagates back to the initial managed caller The problem is as follows: * When a native exception passes through managed frames and it is rethrown once a native frame is encountered, the RaiseException doesn't use the original exception's `EXCEPTION_RECORD`, but creates a new CLR exception instead. * When a managed callback called from the native C++ catch handler throws and catches another exception, the LastThrownObject is cleared. * When the C++ code in the catch handler rethrows the exception and it is propagated to managed code, the ProcessCLRException personality routine is invoked for the first managed frame. It can see that the exception is a CLR exception, but the LastThrownObject is NULL. It asserts in debug builds and crashes a bit later in release builds attempting to dereference the NULL. The fix is to pass the original `EXCEPTION_RECORD` to the `RaiseException` in case an external exception is propagated. And use just `RaiseException` instead of the machinery for regular managed exceptions. The issue was discovered in Autodesk Revit 2025 that does this kind of exception propagation. Close dotnet#113158
There was a problem hiding this comment.
PR Overview
This PR addresses issues with foreign exception handling on Windows by preserving the original EXCEPTION_RECORD during propagation through managed and native frames.
- Introduces a new test to simulate the exception propagation scenario between managed and native code.
- Validates the rethrow behavior and exception logging for backward compatibility with legacy exception handling.
Reviewed Changes
| File | Description |
|---|---|
| src/tests/baseservices/exceptions/PropagateThroughNative/PropagateThroughNative.cs | New test file to verify proper exception propagation across managed/native boundaries |
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/tests/baseservices/exceptions/PropagateThroughNative/PropagateThroughNative.cs:38
- Consider adding assertions to verify the expected exception propagation behavior instead of relying solely on console output. This can help ensure that the test fails when the behavior deviates from expectations.
[Fact]
|
cc: @mangod9, @jeffschwMSFT |
Does our documentation have a mention of whatever the problematic pattern is here? Similar to what we say for setjmp/longjmp: https://learn.microsoft.com/en-us/dotnet/standard/native-interop/exceptions-interoperability#setjmplongjmp-behaviors |
|
@jkotas looking that that doc, it seems we still consider native exception interop as something supported. I have thought we've discussed that in the past as something legacy, guiding people to not to propagate exceptions over the native / managed boundaries. |
|
@jkotas one more thing. While investigating the issue, I have found a similar case that crashes with both the legacy and new EH. The scenario is almost the same except that instead of the C++ exception there is a managed exception propagated through the same kind of frames. The crash is again due to the |
|
If it's a simple enough fix guess we should fix that case too. Agree to only backport the regression. |
Right, we try to discourage it, but it is supported on Windows.
Right.
This issue was reported by our compat lab on .NET 10. Backport would make sense only if it is customer impacting on .NET 9. Do we have any reports? |
src/tests/baseservices/exceptions/PropagateThroughNative/PropagateThroughNative.cs
Outdated
Show resolved
Hide resolved
|
I have found a problem with the fix when I've migrated my test to the exception interop test suite as @jkoritzinsky has suggested. The problem is that when the throw throws an exception by value, the exception is allocated in the throwing frame. But when the propagate exception back to native frames, we physically unwind the stack and throw from just below the native frame. That means that the exception object location is now in an unwound part of the stack and it is not valid anymore. The original test I had was working ok, as I was throwing just an int and its slot was not overwritten. But after moving the the exception interop tests, I've changed my test to throw and catch the |
|
Fortunately, I have found another better way to fix the problem with the physical stack unwinding. C++ exception handling uses the same thing. The |
|
I've added a commit with a fix for the problem with C++ exception object being in a reclaimed part of stack. |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There is a problem in a special case with the new
exception handling:
The problem is as follows:
EXCEPTION_RECORD, but creates a new CLR exception instead.The fix is to pass the original
EXCEPTION_RECORDto theRaiseExceptionin case an external exception is propagated. And use justRaiseExceptioninstead of the machinery for regular managed exceptions.This is yet another case of a fix for backward compatibility w.r.t. the legacy exception handling.
The issue was discovered in Autodesk Revit 2025 that does this kind of exception propagation.
Close #113158