[Interpreter] A thread abort raised again after a catch skips the catch of the enclosing try #135871

TL;DR: When an interpreted catch handles a ThreadAbortException and the abort is raised again at its end, the catch of an enclosing try does not run in two shapes, although the JIT intends it to. The interpreter looks handlers up at the instruction before the IP where the catch resumes, and in both shapes that instruction is outside the outer try.

Description

When interpreted code catches a ThreadAbortException and the abort is raised again at the end of the catch, the catch of an enclosing try does not run in two shapes:

• the inner catch's leave also exits the enclosing try;
• the inner catch returns to the first instruction of the enclosing try, such as a loop head.

The abort leaves the method instead. The JIT intends the enclosing catch to run (see Expected behavior).

This needs the runtime change from #135520 to reproduce. On Checked main at 2165605, these repros stop at the ReadyForAsyncException assert described in #135463 before the enclosing catch's behavior can be observed. #135520, which fixes that interpreter failure, is still open.

I don't know whether the interpreter means to follow the JIT here. If it does, I'm happy to work on a fix; a possible approach is under Other information.

Reproduction Steps

using System;
using System.Runtime;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;

#pragma warning disable SYSLIB0046 // ControlledExecution is obsolete

public static class Program
{
    static volatile bool s_ready;
    static volatile int s_counter;
    static string s_trace;

    [MethodImpl(MethodImplOptions.NoInlining)]
    static void RunInfiniteLoop()
    {
        s_ready = true;
        while (true)
        {
            if ((++s_counter & 0xfffff) == 0)
                Thread.Sleep(0);
        }
    }

    // The inner catch leaves both trys
    [MethodImpl(MethodImplOptions.NoInlining)]
    static void LeaveOutOfOuterTry()
    {
        try
        {
            try
            {
                RunInfiniteLoop();
            }
            catch (ThreadAbortException)
            {
                s_trace += "inner ";
                goto Done;
            }
        }
        catch (ThreadAbortException)
        {
            s_trace += "outer ";
        }
    Done:
        return;
    }

    // The inner catch returns to the start of the outer try (the loop head)
    [MethodImpl(MethodImplOptions.NoInlining)]
    static void CatchInLoop()
    {
        int count = 0;
        try
        {
            while (true)
            {
                try
                {
                    RunInfiniteLoop();
                }
                catch (ThreadAbortException)
                {
                    s_trace += "catch ";
                    if (++count == 3)
                        break;
                }
            }
        }
        catch (ThreadAbortException)
        {
            s_trace += "outer ";
        }
    }

    static void Run(string name, Action action)
    {
        s_trace = "";
        s_ready = false;
        var cts = new CancellationTokenSource();
        Task.Run(() => { while (!s_ready) Thread.Sleep(10); cts.Cancel(); });
        string result = "no exception";
        try
        {
            ControlledExecution.Run(action, cts.Token);
        }
        catch (Exception e)
        {
            result = e.GetType().Name;
        }
        Console.WriteLine($"{name}: \"{s_trace}\" {result}");
    }

    static int Main()
    {
        Run(nameof(LeaveOutOfOuterTry), LeaveOutOfOuterTry);
        Run(nameof(CatchInLoop), CatchInLoop);
        return 100;
    }
}

Run it with corerun on a Checked runtime built from main with #135520 applied, with DOTNET_InterpMode=1 or DOTNET_InterpMode=3.

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <RequiresProcessIsolation>true</RequiresProcessIsolation>
    <ReferenceXUnitWrapperGenerator>false</ReferenceXUnitWrapperGenerator>
    <CLRTestPriority>1</CLRTestPriority>
    <CLRTestTargetUnsupported Condition="'$(RuntimeFlavor)' != 'coreclr'">true</CLRTestTargetUnsupported>
    <NativeAotIncompatible>true</NativeAotIncompatible>
    <Optimize>true</Optimize>
  </PropertyGroup>
  <ItemGroup>
    <Compile Include="InterpAbortOuterCatch.cs" />
  </ItemGroup>
</Project>

# From the repository root, with #135520's runtime change applied:
./build.sh clr+libs -rc Checked -lc Release
src/tests/build.sh checked x64 generatelayoutonly /p:LibrariesConfiguration=Release
src/tests/build.sh checked x64 -priority1 -skipnative -skipgeneratelayout -test:Regressions/coreclr/InterpAbortOuterCatch/InterpAbortOuterCatch.csproj /p:LibrariesConfiguration=Release
export CORE_ROOT="$PWD/artifacts/tests/coreclr/linux.x64.Checked/Tests/Core_Root"
cd artifacts/tests/coreclr/linux.x64.Checked/Regressions/coreclr/InterpAbortOuterCatch/InterpAbortOuterCatch
DOTNET_InterpMode=1 "$CORE_ROOT/corerun" InterpAbortOuterCatch.dll
# Repeat with DOTNET_InterpMode=3. The program intentionally returns 100.

Expected behavior

LeaveOutOfOuterTry: "inner outer " OperationCanceledException
CatchInLoop: "catch outer " OperationCanceledException

On main, the JIT gives inner outer for LeaveOutOfOuterTry with DOTNET_TieredCompilation=0, and catch outer for CatchInLoop with default settings. In other configurations the JIT gives different results on these shapes (#135637, with a proposed fix in #135638). For the first shape, impImportLeave states the intent and makes the catch return to a step block inside the enclosing try (importer.cpp):

If this exception return address skips an enclosing try/catch that catches ThreadAbortException, then the enclosing try/catch will not catch the exception, as it should.

[...]

Note that this pattern isn't theoretical: it occurs in ASP.NET, in IL code generated by the Roslyn C# compiler.

Actual behavior

LeaveOutOfOuterTry: "inner " OperationCanceledException
CatchInLoop: "catch " OperationCanceledException

with DOTNET_InterpMode=1 and 3, three runs each on linux-x64 and on osx-arm64, all identical. The abort still reaches the caller of ControlledExecution.Run; what is missing is the outer catch.

When the interpreter raises the abort again, it treats the IP where the catch resumes as a return address, so handlers are looked up at the instruction before that IP (#135520 keeps this). In both shapes that instruction is outside the outer try. DOTNET_InterpDump with DOTNET_InterpMode=1 (excerpts, comments mine):

LeaveOutOfOuterTry
0018: IR_0004: call           [0 <- 16], .Program:RunInfiniteLoop()
0028: IR_0008: br             [nil <- nil], IR_000c
0030: IR_000a: leavecatch     [nil <- nil], IR_0010    ; end of the inner catch
0038: IR_000c: br             [nil <- nil], IR_0010
0040: IR_000e: leavecatch     [nil <- nil], IR_0010    ; end of the outer catch
0048: IR_0010: ret.void       [nil <- nil],
  try [IR_0004(0), IR_000a(7)) catch handler [IR_0011(7), IR_002a(1e))
  try [IR_0004(0), IR_000e(20)) catch handler [IR_002a(20), IR_0043(37))

CatchInLoop
0024: IR_0007: mov.4          [0 <- 16],               ; count = 0
0030: IR_000a: call           [16 <- 32], .Program:RunInfiniteLoop()
0040: IR_000e: br             [nil <- nil], IR_000a
0048: IR_0010: leavecatch     [nil <- nil], IR_0014    ; break
0050: IR_0012: leavecatch     [nil <- nil], IR_000a    ; end of the inner catch, back to the loop head
  try [IR_000a(3), IR_0010(a)) catch handler [IR_0019(a), IR_004b(2b))
  try [IR_000a(2), IR_0016(2d)) catch handler [IR_004b(2d), IR_0064(44))

• LeaveOutOfOuterTry: the inner catch resumes at IR_0010, past the outer try [IR_0004, IR_000e). CreateLeaveChainIslandBasicBlocks creates a leave island for each catch handler and each finally-protected try that a leave exits. It creates none for a try protected by a catch. So the catch returns directly to IR_0010 rather than to a continuation inside the outer try.
• CatchInLoop: the inner catch resumes at the loop head IR_000a, the first instruction of both trys, so the instruction before it is outside both. Excluding a try that the catch's continuation is about to enter is consistent with [Interpreter] Raise a pending thread abort again after a catch handler #135520. Here the outer try already encloses the catch, while the inner try belongs to the next loop iteration, so the abort should reach the outer catch before that iteration starts.

Other shapes I tried give the expected handlers. For example, with a finally between the inner and the outer catch, the finally runs before the outer catch.

Regression?

Not that I know of. Main does not get this far without #135520.

Known Workarounds

None known while running under the interpreter.

Configuration

Checked linux-x64 and Checked osx-arm64, main at 2165605 with the runtime change of #135520 at 7e70fe0 (it applies without conflicts). Both give the same interpreter output and the same instruction layout and EH region bounds in DOTNET_InterpDump. Without #135520, every interpreter run of the program stops at the assert of #135463 (ExecutionManager::IsManagedCode(GetIP(m_OSContext))).

Other information

Found while validating #135638, the JIT fix for #135637. Its regression test checks which handlers run in shapes like these on jitted code, and I'll make it skip the interpreter with an [ActiveIssue] that points here. If the interpreter is meant to follow the JIT, a possible fix is to give the catch a resume point whose preceding instruction is inside the enclosing try but outside the inner try. Handlers are looked up at that preceding instruction, so this keeps the enclosing coverage that the JIT's step block provides. I have not tried it.