Exception stack not cleaned up after return inside of a catch handler with a finally clause

[twhitehead]

I ran into issues with the exception test suite failing in a particular build environment when ran in the complete test suite but not by itself.

Through a process of elimination I eventually traced the issue down to test12806 in core.jl leaving things on the exception stack.

The following error.jl code, which is just core test12806 followed by the very first exception test, seems to reliably demonstrate the issue for both 1.3.0 and 1.3.1 across several linux distributions (nix, gento, arch, debian).

using Test
using Base: catch_stack

function test12806()
    let catchb = false, catchc = false, catchr = false, a = []
        for i in 1:3
            try
                throw("try err")
            catch e
                i == 1 && break
                i == 2 && continue
                i == 3 && return (catchb, catchc, catchr, a)
            finally
                i == 1 && (catchb = true; continue)
                i == 2 && (catchc = true; )
                i == 3 && (catchr = true; push!(a, 1))
            end
        end
    end
end
@test test12806() == (true, true, false, [1])
@test length(catch_stack()) == 0

Here is the output I get from running it

$ julia error.jl
Test Failed at /home/tyson/julia/error.jl:22
  Expression: length(catch_stack()) == 0
   Evaluated: 1 == 0
ERROR: LoadError: There was an error during testing
in expression starting at /home/tyson/julia/error.jl:22
caused by [exception 1]
"try err"
Stacktrace:
 [1] test12806() at /home/tyson/julia/error.jl:8
 [2] top-level scope at /home/tyson/julia/error.jl:21
 [3] include_relative(::Module, ::String) at /nix/store/0xhszcg5kaw5kl4z2lyhnwsb2b9k8z27-julia-1.3.0/lib/julia/sys.so:?
 [4] include(::Module, ::String) at /nix/store/0xhszcg5kaw5kl4z2lyhnwsb2b9k8z27-julia-1.3.0/lib/julia/sys.so:?
 [5] exec_options(::Base.JLOptions) at /nix/store/0xhszcg5kaw5kl4z2lyhnwsb2b9k8z27-julia-1.3.0/lib/julia/sys.so:?
 [6] _start() at /nix/store/0xhszcg5kaw5kl4z2lyhnwsb2b9k8z27-julia-1.3.0/lib/julia/sys.so:?

[twhitehead]

CCing @JeffBezanson as commit history indicates you added the test case 12806.

[twhitehead]

Spent a bit of time trying to simplify this down further. Looks like the actual issue is that the stack doesn't clean up when you do a return inside of a catch block with a finally clause as demonstrated in this error.jl code

using Base: catch_stack

function foo()
  try
    throw("err")
  catch
    return
  finally
  end
end

foo()
println(length(catch_stack()))

$ julia error.jl
1
$

Seems this isn't actually tied to break or continue after all. I'll updated the bug title.

[twhitehead]

I believe the unique thing in the build environment I've been working in that triggered this (and other) test interaction errors to appear is the fact that it runs isolated from the network.

From looking at the code, I believe the lack of a network causes julia to run the test sequentially one on worker. This means that all tests share some global state and causes issues between each other. I would presume whether these issues occur normally or not will otherwise depend on which tests run on which workers.

I expect you can test for these failures in a non-isolated build environment by manually hacking test/runtests.jl

cd(@__DIR__) do
    n = 1
    if net_on
        n = min(Sys.CPU_THREADS, length(tests))
        n > 1 && addprocs_with_testenv(n)
        LinearAlgebra.BLAS.set_num_threads(1)
    end
    skipped = 0

to not set n>1 (e.g., change net_on to false).

[c42f]

Very likely this is a bug in lowering.

Syntax desugaring turns the try-catch-finally into the nested form

function foo()
  try
    try
      throw("err")
    catch
      return
    end
  finally
  end
end

which is then transformed by compile-body in a later lowering pass, with some rather tricky logic to handle the possible combinations of try/catch/finally/break/continue/return.

The following form does not have this problem, so it's some interaction with the finally block where returning from within the catch must run the finally code.

function foo()
    try
      throw("err")
    catch
      return
    end
end

[c42f]

I spent a while staring at the applicable part of lowering.

This is a bug in how lowering's linearization pass for exception stack pop_exception interacts with lowering of finally blocks.

Specifically, the lowering of finally is quite subtle when combined with break or return because each finally block may be entered via multiple code paths (eg, different occurrences of return), and these code paths must diverge again once the finally block has completed. To complicate matters further, the return code path must thread through every nested finally block before actually returning, all the while preserving the information about which variable to return.

To get all this to work there's a rather neat system for tagging the return path and emitting the right code to leave the inner scope of any exception handlers, etc. But the code for popping the exception stack is missing (for return-via-finally) or subtly broken (for break-via-finally).

Here's a related failing test for break-via-finally which pops the exception stack too early, so is less serious but still wrong:

function foo()
    while true
        try
            throw("Expected")
        catch
            try
                break
            finally
                @test length(Base.catch_stack()) == 1
            end
        end
    end
end