[wasm] Thread abort is raised inside finally and catch handlers of R2R code, so ControlledExecution.Run lets ThreadAbortException escape #135914

Description

On browser-wasm CoreCLR with the default ReadyToRun CoreLib, ControlledExecution.Run does not turn the abort into an OperationCanceledException. The abort is raised again inside Run's own finally and catch (ThreadAbortException), both R2R code, before the catch can throw.

using System;
using System.Runtime;
using System.Threading;

#pragma warning disable SYSLIB0046
var cts = new CancellationTokenSource();
try
{
    ControlledExecution.Run(() => { cts.Cancel(); while (true) { } }, cts.Token);
}
catch (Exception e)
{
    Console.WriteLine(e.GetType()); // expected: System.OperationCanceledException
}

I measured the following outcomes in a harness equivalent to the snippet, using browser-wasm Checked runtimes under node (corerun.js), single-threaded. The PR-head runs used 7e70fe0dd4db plus a diagnostic marker. Cancel() invokes the abort callback synchronously on the current thread; the spin after it is not how the abort is delivered.

The same happens without a CancellationTokenSource callback in between, when the thread aborts itself through ControlledExecution's private Canceler.Cancel() (reflection). On main, Run throws ThreadAbortException instead of OperationCanceledException, both with an action that has no handler at all and with one that has only a try/finally; with DOTNET_ReadyToRun=0 it throws OperationCanceledException. I did not observe these in-handler aborts in the linux-x64 JIT and interpreter configurations I tested (with the interpreter and R2R CoreLib the token case fails differently, #135532), or on wasi at #135520's head with an interpreted CoreLib.

Analysis

In the cold runs traced with DOTNET_LogEnable=1 DOTNET_LogFacility=0x4000 DOTNET_LogLevel=10 DOTNET_LogToConsole=1, the abort is raised again at calls through unbound R2R import cells in Run's handlers: Registrations.Unregister in the inner finally, CancellationToken.IsCancellationRequested in the catch, and ResetAbortThread in the outer finally (so the reset never runs). CancellationTokenSource's own catch and finally are interrupted the same way. Warming an empty Run first removes the Unregister poll, but the call in the catch still raises the abort.

Thread::ReadyForAsyncException refuses to abort inside a finally or catch handler. Its walk, TAStackCrawlCallBackWorker, moves a non-active frame's offset back by one because "our IP is a return address". On wasm an R2R frame's control PC is a virtual IP: fgWasmVirtualIP numbers regions, and every call in a handler's first region reports exactly the handler's start. Moved back by one, that is outside the handler, so the walk decides the thread is not in a handler. The log (main and #135520's head alike) shows Run frames at offsets 0x10, 0x14 and 0x18, the starts of its three handlers, checked as 0xf, 0x13 and 0x17. SfiInit and SfiNext already skip this adjustment for virtual IPs (ExecutionManager::IsVirtualIP); TAStackCrawlCallBackWorker does not.

Skipping the adjustment for virtual IPs (change below) makes the self-abort cases above pass. The CancellationTokenSource case then reaches a second problem: at the end of CTS's R2R catch, EECodeManager::ResumeAfterCatch copies the resume context into m_OSContext and calls COMPlusCheckForAbort, and ReadyForAsyncException asserts IsManagedCode(GetIP(m_OSContext)). On wasm that IP is the placeholder zero returned by the catch funclet, not a code address; the catch-resume index is stored separately in the frame. From source, RtlRestoreContext on wasm only throws the restore tag and does not use the context, so the SetIP(pContext, uAbortAddr) used for the end-of-catch abort redirect would not redirect execution. I have not measured Release behavior. Without the virtual-IP change, the CTS runs I traced raise the abort inside CTS's catch before it reaches this path.

         // may be in an cloned finally which has a call as the last instruction.

-        offs--;
+#ifdef TARGET_WASM
+        // On Wasm, R2R code with virtual ips should not have its ip adjusted
+        if (!ExecutionManager::IsVirtualIP(GetControlPC(pCf->GetRegisterSet())))
+#endif
+        {
+            offs--;
+        }
     }

I think this is the fix for the first problem, not only a diagnostic. fgWasmVirtualIP starts a new region at every try, handler and filter boundary and stores the current region before each call, so a frame's virtual IP is the region of the call in progress, not a return address. The VM doubles both the frame's offset and the clause bounds, so the -1 evaluates the previous region. For this walk, which checks only handler ranges, that changes the answer only at a handler's first region (this bug): crossing a try boundary, in the main body or within a handler, does not change handler membership. No main-body region follows a handler, because handlers are funclets numbered after the main body. With the change, a copy of ControlledExecutionTests.CancelBeforeRun also passes on browser-wasm, where it fails on main and at #135520's head. The change does not address the second problem.

ControlledExecutionTests does not run on wasm today: the class is conditional on IsMultithreadingSupported, although only four of its enabled tests use Task.Run. I'm working on a PR for the first problem with the change above, moving that condition onto those four tests so that CancelBeforeRun and the others run on single-threaded wasm.

For the second problem I would like your direction before turning the local prototype into a proposed fix. Would you take the end-of-catch abort check into the resumed code on wasm? The JIT would always emit CORINFO_HELP_JIT_RESUME_AFTER_CATCH at catch resumption pads (today only with --verify-gc-mode-transitions), and JIT_ResumeAfterCatch would check for a pending abort from its caller's frame and raise it there, as #135520 does for the interpreter. A local prototype of that, combined with the change above and #135520, removes both failures in my 23 browser cases (the handler-trace differences #135520 already has remain), but it relies on the Checked CoreLib being built with that flag. Or would you rather keep the check in the VM? Then it needs a way to learn the continuation's region, which the JIT does not report today.

Found while validating #135520 on wasm. Related: #135463, #135532.