[Interpreter] Raise a pending thread abort again after a catch handler

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

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();

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<OperationCanceledException>(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 ");
+    }
+}

src/tests/Regressions/coreclr/GitHub_135463/test135463.csproj

@@ -0,0 +1,11 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <CLRTestPriority>1</CLRTestPriority>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="test135463.cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <ProjectReference Include="$(TestLibraryProjectPath)" />
+  </ItemGroup>
+</Project>