Infinite assert loop with try/finally

[Herringway]

void main() {
    try assert(0);
    finally assert(0);
}

With DMD 2.112, this will print far more (possibly infinitely more) than the expected two stack traces, at least on linux. On windows, it appears to hang instead.

The same result is observed with the equivalent in scope statements:

void main() {
    scope(exit) assert(0);
    assert(0);
}

This problem also exists in GCC 16.2.1 and LDC 1.42.0, pointing to a frontend issue. I could not test if this was a regression - run.dlang.io REALLY does not like running this program, unsurprisingly.

[limepoutine]

Appears to be a druntime issue as --DRT-trapExceptions=0 prevents the loop.

This behavior is prohibited in statements spec, section 12.20.6 and 12.21.2. What to do with assertions, I have no idea.

If an exception is raised in the FinallyStatement and is not caught before the original exception is caught, it is chained to the previous exception via the next member of Throwable. Note that, in contrast to most other programming languages, the new exception does not replace the original exception. Instead, later exceptions are regarded as 'collateral damage' caused by the first exception. The original exception must be caught, and this results in the capture of the entire chain.

A scope(exit) or scope(success) statement may not exit with a throw, goto, break, continue, or return; nor may it be entered with a goto. A scope(failure) statement may not exit with a return.

[CaradasProudfoot]

assert(0) in both cases should be considered as a block terminator, and consequently there should be the same errors as for

void a() {
    try assert(0);
    finally return; // Error: `return` statements cannot be in `finally` bodies

}

void b() {
    scope(exit) return; // Error: `return` statements cannot be in `scope(exit)` bodies
    assert(0);
}

[limepoutine]

The same bug would still manifest with assertions on a run-time value.

bool glorifiedFalse;

void main() {
    try assert(glorifiedFalse);
    finally assert(glorifiedFalse);
}

[limepoutine]

This is a core.exception.staticError reference cycle:

__gshared Error e;

void f() {
    throw e;
}

void main() {
    e = new Error("");
    try f();
    finally f();
}

[ibuclaw]

This is a core.exception.staticError reference cycle:

Yep, introduced by dlang/druntime@5660983 (@Geod24)

You can confirm this by overriding onAssertError with the previous behaviour.

extern (C) void onAssertError(string file, size_t line) nothrow
{
    import core.exception : AssertError;
    throw new AssertError(file, line);
}
void main()
{
    try assert(0);
    finally assert(0);
}

[Herringway]

assert(0) in both cases should be considered as a block terminator, and consequently there should be the same errors as for

void a() {
try assert(0);
finally return; // Error: return statements cannot be in finally bodies

}

void b() {
scope(exit) return; // Error: return statements cannot be in scope(exit) bodies
assert(0);
}

The problem also manifests for more complicated examples:

int[] arr;
int foo() => arr[0];
void main() {
  scope(exit) foo();
  foo();
}

So I don't think a compiler error is going to be the answer. My initial examples were just the simplest form of the problem.

[Herringway]

This is a core.exception.staticError reference cycle:

Yep, introduced by dlang/druntime@5660983 (@Geod24)

You can confirm this by overriding onAssertError with the previous behaviour.

extern (C) void onAssertError(string file, size_t line) nothrow
{
import core.exception : AssertError;
throw new AssertError(file, line);
}
void main()
{
try assert(0);
finally assert(0);
}

Doesn't seem to actually work (windows-dmd, windows-ldc, linux-dmd, linux-gdc, linux-ldc with the latter two throwing linker errors), but setting assertHandler does:

void main() {
	import core.exception : AssertError, assertHandler;
	assertHandler = (file, line, msg) { throw new AssertError(msg, file, line); };
	scope(exit) assert(0);
	assert(0);
}

[Herringway]

So if I understand the root cause here, it's that the AssertError being thrown is chained to itself due to a faulty assumption that there will only ever be one instance of an AssertError active at any given time?

[ibuclaw]

So if I understand the root cause here, it's that the AssertError being thrown is chained to itself due to a faulty assumption that there will only ever be one instance of an AssertError active at any given time?

Yes. The store in staticError is thrown, unwind handlers jump back into the finally block and store is reinitialised and thrown again whilst there's still live references to it. In-flight detection then links the two together, creating a loop.

[ibuclaw]

This is a core.exception.staticError reference cycle:

__gshared Error e;

void f() {
    throw e;
}

void main() {
    e = new Error("");
    try f();
    finally f();
}

As per this, there are a couple things going on here. But all reduce to code that throws the same Throwable object (reference pointer) more than once whilst unwind is running through all finallly blocks.

Who's problem is this?

1. User code error.
2. Compiler allowing throw in a finally block.
3. Runtime onAssertError and others using the same storage for each others Error objects.
4. Runtime staticError not iterating through an array of static storage to reduce chance of emplacing on live reference.
5. Runtime Throwable.chainTogether(a,b) not checking if b already appears in as linked list.
6. Runtime backtrace printers not having infinite loop detection.

Any other's I've missed?

I think (6) should be dealt with first. Bail with recursive chain detected and exit is better than printing the same N Throwables forever.

[Herringway]

This is a core.exception.staticError reference cycle:
__gshared Error e;

void f() {
throw e;
}

void main() {
e = new Error("");
try f();
finally f();
}

As per this, there are a couple things going on here. But all reduce to code that throws the same Throwable object (reference pointer) more than once whilst unwind is running through all finallly blocks.

Who's problem is this?

1. User code error.

It's rare to see an Error that ISN'T a user code error ;D

2. Compiler allowing `throw` in a finally block.

I think forbidding this would either be ineffective to the point of being a total waste of time or restrictive enough to make finally (and scope(exit)) useless - we can't be sure if a function doesn't throw anything, even if it's nothrow

3. Runtime `onAssertError` and others using the same storage for each others Error objects.

4. Runtime `staticError` not iterating through an array of static storage to reduce chance of emplacing on live reference.

This and (3) should definitely be addressed. The real code I'm dealing with threw a bunch of AssertErrors and other flavours of Error in a bunch of scope(exit)s meant for cleanup after I had accidentally broken a core data structure, and having the last AssertError overwrite the one I'm most interested in made diagnosis harder than it needed to be.

5. Runtime `Throwable.chainTogether(a,b)` not checking if `b` already appears in `a`s linked list.

Certainly nice to have.

6. Runtime backtrace printers not having infinite loop detection.

Seems like a good low-hanging fruit to deal with.

Any other's I've missed?

I think (6) should be dealt with first. Bail with recursive chain detected and exit is better than printing the same N Throwables forever.

Agreed. 3+4 should follow it.

[ibuclaw]

I think (6) should be dealt with first. Bail with recursive chain detected and exit is better than printing the same N Throwables forever.

Actually, I partially retract this. The bug is definitely in Throwable.chainTogether - though loop detection in backtrace is still a good thing (but it's an Internal Runtime Error: ...)

Consider this:

try
{
    try throw ex1;
    finally throw ex1;  // chainTogether creates infinite loop ex1->ex1.
}
finally throw ex2;      // chainTogether hangs whilst trying to append `ex2` to end of `ex1`.