[Issue 23555] New: Throwing an Error in a destructor hangs on a collection

[d-bugmail at puremagic.com]

https://issues.dlang.org/show_bug.cgi?id=23555

          Issue ID: 23555
           Summary: Throwing an Error in a destructor hangs on a
                    collection
           Product: D
           Version: D2
          Hardware: x86
                OS: Mac OS X
            Status: NEW
          Severity: enhancement
          Priority: P1
         Component: druntime
          Assignee: nobody at puremagic.com
          Reporter: schveiguy at gmail.com

If you throw an error in a destructor, the GC will hang if it tries to run that
destructor.

```d
class C
{
    ~this()
    {
        assert(false, "bad");
    }
}

void main()
{
   new C;
}
```

If compiled in non-release mode, this will hang.

What happens is the assert is thrown and *not* caught by the code in druntime
that prevents exceptions from getting to the GC runFinalizers routine. If you
change the assert to a throw of a staticaally-allocated exception, you instead
get a nice error message and exit of the program.

The "offending" PR is here: https://github.com/dlang/druntime/pull/1447

The issue is that there is code such as:
https://github.com/dlang/druntime/pull/1447/files#diff-ded8b221f7392e8cfea15564dc107aa3007a832a7debadc3755559c4121f799eR2446-R2452

What happens? The destructor throws the assert error.

Upon throwing, the defaultTraceHandler function here:
https://github.com/dlang/dmd/blob/fabd06214ef699279bdbb83a13bd8d1be34e2e34/druntime/src/core/runtime.d#L676

will avoid allocating because it's in a finalizer. Great, this avoids the
deadlock.

But once it reaches the `scope(failure)` clause, it's caught, and rethrown
*after the inFinalizer flag is turned off*. This means that it now will attempt
to allocate a traceinfo, which tries to take the lock a second time, and then
enters a deadlock state.

The ultimate solution is that defaultTraceinfoHandler should not use the GC. In
all honesty, the allocated data for the traceinfo can easily be obtained using
C malloc calls. This is the real solution.

But the scope failure clause should be remedied as well. If you are going to
unset the flag, you should unlock the lock as well. If you aren't going to
unlock the lock, the unsetting of the flag should probably come later (or not
at all, this particular scope(failure) can't possibly happen with an Exception,
as the function is nothrow. I haven't thought through the correct remedy.

The code is currently here:
https://github.com/dlang/dmd/blob/020685c85b4fde7d50511716dc98dfc5dc97ef2b/druntime/src/core/internal/gc/impl/conservative/gc.d#L3137

There are other scope(failure) calls in that file that might need attention.

--