Thread overview
Struct field destructor not called when exception is thrown in the main struct destructor
Oct 16, 2020
tchaloupka
Oct 16, 2020
Ali Çehreli
Oct 16, 2020
Ali Çehreli
Oct 16, 2020
Paul Backus
Oct 16, 2020
Paul Backus
Oct 17, 2020
tchaloupka
October 16, 2020
Found a pretty nasty bug in vibe-d: https://github.com/vibe-d/vibe.d/issues/2484

And it's caused by this behavior.

```D
import std;

struct Foo {
    Bar bar;
    bool err;

    ~this() {
        // scope(failure) destroy(bar); // < this fixes the Bar destructor call
        enforce(!err, "Test err");
    }
}

struct Bar {
    static int refs;
    ~this() { refs--; }
}

void main()
{
    {
        Foo f;
        Bar.refs = 1;
    }
    assert(Bar.refs == 0);

    try () {
        Foo f;
        f.err = true;
        Bar.refs = 1;
    }();
    catch (Exception ex) {}
    assert(Bar.refs == 0);
}
```

So when the exception is thrown within Foo destructor (and it's bad on it's own but can easily happen as destructors aren't nothrow @nogc by default).

Is this behavior expected?
October 16, 2020
On 10/16/20 6:12 AM, tchaloupka wrote:

> struct Foo {
>      Bar bar;
>      bool err;
>
>      ~this() {
>          // scope(failure) destroy(bar); // < this fixes the Bar
> destructor call
>          enforce(!err, "Test err");

Well, that check means "cannot continue", which means the compiler stops executing the destruction code because it can't. (It would a serious bug if it continued execution in a state that the program knows to be invalid.)

>      }

Conceptually, bar's destructor is called on that closing brace but we decided to abort mission earlier. Your calling destroy(bar) may or may not be wrong in case of 'err'. Only you know at that point.

> Is this behavior expected?

Yes.

Ali

October 16, 2020
On Friday, 16 October 2020 at 13:12:04 UTC, tchaloupka wrote:
> So when the exception is thrown within Foo destructor (and it's bad on it's own but can easily happen as destructors aren't nothrow @nogc by default).
>
> Is this behavior expected?

This is a compiler/language bug. It was fixed in DMD 2.083.0, but the new behavior requires a -preview switch, since it has the potential to break code.

More information:

- https://dlang.org/changelog/2.083.0.html#reboot14246
- https://dlang.org/changelog/2.085.0.html#preview-flags
October 16, 2020
On Friday, 16 October 2020 at 15:19:51 UTC, Paul Backus wrote:
> On Friday, 16 October 2020 at 13:12:04 UTC, tchaloupka wrote:
>> So when the exception is thrown within Foo destructor (and it's bad on it's own but can easily happen as destructors aren't nothrow @nogc by default).
>>
>> Is this behavior expected?
>
> This is a compiler/language bug. It was fixed in DMD 2.083.0, but the new behavior requires a -preview switch, since it has the potential to break code.
>
> More information:
>
> - https://dlang.org/changelog/2.083.0.html#reboot14246
> - https://dlang.org/changelog/2.085.0.html#preview-flags

Oops, I misread the destructor as a constructor in the original post. Never mind!
October 16, 2020
On 10/16/20 9:12 AM, tchaloupka wrote:

> So when the exception is thrown within Foo destructor (and it's bad on it's own but can easily happen as destructors aren't nothrow @nogc by default).
> 
> Is this behavior expected?

I would say it's a bug. The compiler is going to call the member destructor even if the hand-written destructor does it too. If the compiler wants to take responsibility for cleaning up members, it should take full responsibility. In fact, there is no way to instruct the compiler "I'm handling the destruction of this member", so I don't see why it should matter if you exit the function via exception it should be any different.

-Steve
October 16, 2020
On 10/16/20 11:11 AM, Ali Çehreli wrote:
> On 10/16/20 6:12 AM, tchaloupka wrote:
> 
>  > struct Foo {
>  >      Bar bar;
>  >      bool err;
>  >
>  >      ~this() {
>  >          // scope(failure) destroy(bar); // < this fixes the Bar
>  > destructor call
>  >          enforce(!err, "Test err");
> 
> Well, that check means "cannot continue", which means the compiler stops executing the destruction code because it can't. (It would a serious bug if it continued execution in a state that the program knows to be invalid.)

The destruction of members is outside the destructor's purview. It can't turn the destruction off, so it should logically be considered part of an enclosing function.

Note that enforce is throwing an *Exception*, not an *Error*, it does not mean the program is in an invalid state. Throwing an Error, the compiler is allowed to not clean up after itself.

Imagine if throwing an Exception disabled RAII in any enclosing function scopes because it considered that an invalid state.

> Conceptually, bar's destructor is called on that closing brace but we decided to abort mission earlier. Your calling destroy(bar) may or may not be wrong in case of 'err'. Only you know at that point.

If that is the case, a hand written destructor should turn off automatic destruction of members. Either the compiler handles member destruction or it doesn't.

-Steve
October 16, 2020
On 10/16/20 9:05 AM, Steven Schveighoffer wrote:

> The destruction of members is outside the destructor's purview. It can't
> turn the destruction off, so it should logically be considered part of
> an enclosing function.

Thank you. Makes sense.

Ali

October 17, 2020
On Friday, 16 October 2020 at 16:00:07 UTC, Steven Schveighoffer wrote:
> On 10/16/20 9:12 AM, tchaloupka wrote:
>
>> So when the exception is thrown within Foo destructor (and it's bad on it's own but can easily happen as destructors aren't nothrow @nogc by default).
>> 
>> Is this behavior expected?
>
> I would say it's a bug. The compiler is going to call the member destructor even if the hand-written destructor does it too. If the compiler wants to take responsibility for cleaning up members, it should take full responsibility. In fact, there is no way to instruct the compiler "I'm handling the destruction of this member", so I don't see why it should matter if you exit the function via exception it should be any different.
>
> -Steve

Thx, added https://issues.dlang.org/show_bug.cgi?id=21322