diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 0e21a28c23b16d..f4c79492b8b880 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -1522,6 +1522,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr { INSTALL_MANAGED_EXCEPTION_DISPATCHER; INSTALL_UNWIND_AND_CONTINUE_HANDLER; + while (true) { #if USE_COMPUTED_GOTO @@ -4942,8 +4943,13 @@ do \ } catch (const ResumeAfterCatchException& ex) { - GCX_COOP_NO_DTOR(); ex.GetResumeContext(&resumeSP, &resumeIP); + goto RESUME_AFTER_CATCH; + } + +RESUME_AFTER_CATCH: + { + GCX_COOP_NO_DTOR(); _ASSERTE(resumeSP != 0 && resumeIP != 0); InterpMethodContextFrame* pResumeFrame = (InterpMethodContextFrame*)resumeSP; @@ -4979,12 +4985,19 @@ do \ pInterpreterFrame->SetIsFaulting(false); - void* abortAddress = COMPlusCheckForAbort(resumeIP); - if (abortAddress != NULL) + // If the thread is being aborted, the abort has to be raised again now that the catch handler is done, like + // EECodeManager::ResumeAfterCatch does for compiled code. + + // Record the resume IP in the pFrame so that the exception handling unwinds from there + pFrame->ip = ip; + + // COMPlusCheckForAbort expects Thread::m_OSContext to describe the place where the execution resumes. It + // uses that context to find out whether the thread can be aborted there. + pInterpreterFrame->SetContextToInterpMethodContextFrame(GetThread()->m_OSContext); + + if (COMPlusCheckForAbort(resumeIP) != NULL) { - // Record the resume IP in the pFrame so that the exception handling unwinds from there - pFrame->ip = ip; - DispatchManagedException(kThreadAbortException); + goto ABORT_AFTER_CATCH; } goto MAIN_LOOP; @@ -5015,6 +5028,25 @@ do \ RETHROW_RESUME_AFTER_CATCH: // Rethrow the exception to let it propagate to the correct resume frame ThrowResumeAfterCatchException(resumeSP, resumeIP); + +ABORT_AFTER_CATCH: + // Raise the abort again at the resume IP. It has a try block of its own: when a frame of this InterpExecMethod + // catches the abort, the ResumeAfterCatchException that resumes that frame is caught here. + try + { + INSTALL_MANAGED_EXCEPTION_DISPATCHER; + INSTALL_UNWIND_AND_CONTINUE_HANDLER; + DispatchManagedException(kThreadAbortException); + UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; + UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; + } + catch (const ResumeAfterCatchException& ex) + { + ex.GetResumeContext(&resumeSP, &resumeIP); + goto RESUME_AFTER_CATCH; + } + + UNREACHABLE(); } #endif // FEATURE_INTERPRETER diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Runtime/ControlledExecutionTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Runtime/ControlledExecutionTests.cs index 4910bead0b75d9..8744fc4d9b1903 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Runtime/ControlledExecutionTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Runtime/ControlledExecutionTests.cs @@ -74,7 +74,7 @@ void Test() // Tests that an infinite loop may be aborted, that the ThreadAbortException is automatically rethrown, // and that it is eventually translated to an OperationCanceledException. -// [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotMonoRuntime), nameof(PlatformDetection.IsNotNativeAot))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotMonoRuntime), nameof(PlatformDetection.IsNotNativeAot))] public void CancelInTryAndExitCatchNormally() { var cts = new CancellationTokenSource(); diff --git a/src/tests/Regressions/coreclr/GitHub_135463/test135463.cs b/src/tests/Regressions/coreclr/GitHub_135463/test135463.cs new file mode 100644 index 00000000000000..a8a6307d4f68ff --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_135463/test135463.cs @@ -0,0 +1,158 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime; +using System.Runtime.CompilerServices; +using System.Threading; +using System.Threading.Tasks; +using TestLibrary; +using Xunit; + +// ControlledExecution.Run is the API that aborts a thread +#pragma warning disable SYSLIB0046 + +public class Runtime_135463 +{ + private static volatile bool s_readyForCancellation; + private static volatile int s_counter; + private static string s_trace; + // Set by a call-free store right after a catch while an abort is pending. A call there (like the string + // concatenation of s_trace) would carry its own safepoint, where the pending abort can be raised even if + // nothing raised it again at the end of the catch. + private static volatile bool s_reachedAfterCatch; + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void RunInfiniteLoop() + { + while (true) + { + if ((++s_counter & 0xfffff) == 0) + { + Thread.Sleep(0); + } + } + } + + private static void WaitUntilAbortIsRequested() + { + while ((Thread.CurrentThread.ThreadState & ThreadState.AbortRequested) == 0) + { + Thread.Sleep(10); + } + } + + // The abort is thrown again at the end of the catch block that swallowed it + [MethodImpl(MethodImplOptions.NoInlining)] + private static void CatchAbort() + { + try + { + s_readyForCancellation = true; + RunInfiniteLoop(); + } + catch (ThreadAbortException) + { + s_trace += "catch "; + } + + s_reachedAfterCatch = true; + } + + // The caller catches the abort that was thrown again at the end of the catch block in its callee + [MethodImpl(MethodImplOptions.NoInlining)] + private static void CatchAbortInCaller() + { + try + { + CatchAbort(); + } + catch (ThreadAbortException) + { + s_trace += "caller catch "; + } + + s_reachedAfterCatch = true; + } + + // A thread is not aborted while it runs a finally block, so the abort stays pending at the end of a catch block + // that is inside of one + [MethodImpl(MethodImplOptions.NoInlining)] + private static void CatchInsideOfFinally() + { + try + { + try + { + throw new InvalidOperationException(); + } + finally + { + s_readyForCancellation = true; + WaitUntilAbortIsRequested(); + + try + { + throw new ArgumentException(); + } + catch (ArgumentException) + { + s_trace += "catch in finally "; + } + + s_trace += "end of finally "; + } + } + catch (InvalidOperationException) + { + s_trace += "outer catch "; + } + + s_reachedAfterCatch = true; + } + + private static void CancelWhenReady(CancellationTokenSource cts) + { + while (!s_readyForCancellation) + { + Thread.Sleep(10); + } + + cts.Cancel(); + } + + // Runs the action, aborts it once it is ready and checks which of its blocks ran + private static void RunAndAbort(string name, Action action, string expectedTrace) + { + Console.WriteLine(name); + s_trace = ""; + s_readyForCancellation = false; + s_reachedAfterCatch = false; + + var cts = new CancellationTokenSource(); + Task.Run(() => CancelWhenReady(cts)); + + Exception exception = null; + try + { + ControlledExecution.Run(action, cts.Token); + } + catch (Exception e) + { + exception = e; + } + + Assert.IsType(exception); + Assert.Equal(expectedTrace, s_trace); + Assert.False(s_reachedAfterCatch); + } + + // ControlledExecution is supported on CoreCLR only + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsCoreCLR), nameof(PlatformDetection.IsMultithreadingSupported))] + public static void TestEntryPoint() + { + RunAndAbort(nameof(CatchAbort), CatchAbort, "catch "); + RunAndAbort(nameof(CatchAbortInCaller), CatchAbortInCaller, "catch caller catch "); + RunAndAbort(nameof(CatchInsideOfFinally), CatchInsideOfFinally, "catch in finally end of finally outer catch "); + } +} diff --git a/src/tests/Regressions/coreclr/GitHub_135463/test135463.csproj b/src/tests/Regressions/coreclr/GitHub_135463/test135463.csproj new file mode 100644 index 00000000000000..a0efe27f10c395 --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_135463/test135463.csproj @@ -0,0 +1,11 @@ + + + 1 + + + + + + + +