D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 704 - `class` destructor is called even if constructor throws
Summary: `class` destructor is called even if constructor throws
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P2 major
Assignee: Walter Bright
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2006-12-21 09:21 UTC by Thomas Kühne
Modified: 2021-12-23 16:15 UTC (History)
8 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Thomas Kühne 2006-12-21 09:21:45 UTC
(originally posted by Sean Kelly <sean@f4.ca> on 2006-01-24 as
 news:dr5uqg$2hn7$1@digitaldaemon.com)

// This demonstrates incorrect behavior for auto class
// construction.  If an exception is thrown out of a
// class ctor then it is an incomplete object and its
// dtor should not be called.

import std.c.stdio;


auto class AutoClass
{
public:
    this()
    {
        printf( "ctor\n" );
        throw new Exception( "" );
    }

    ~this()
    {
        printf( "dtor\n" );
    }
}


void main()
{
    try
    {
        auto AutoClass c = new AutoClass();
    }
    catch( Exception e )
    {
        printf( "caught\n" );
    }
    printf( "continue\n" );
}
C:\code\d\bugs>dmd 101_1.d
C:\bin\dmd\bin\..\..\dm\bin\link.exe 101_1,,,user32+kernel32/noi;

C:\code\d\bugs>101_1
ctor
caught
continue
dtor

test cases:
http://dstress.kuehne.cn/run/a/auto_14_A.d
http://dstress.kuehne.cn/run/a/auto_14_B.d
Comment 1 Brad Roberts 2007-05-09 17:42:28 UTC
Retesting with a newest version of the compiler (1.014), these two tests should probably be altered with s/auto/scope/ due to the changes in the language.

Also, throwing from a destructor is iffy behavior by itself.  Either way, they still hit the throw in the dtor.

However, fixing the throw from destructor issue like this:
     1  module dstress.run.a.auto_14_A;
     2  
     3  import std.stdio;
     4  
     5  bool dtorcalled = false;
     6  
     7  auto class MyClass{
     8      this(){
     9          writefln("in ctor");
    10          throw new Exception("dummy");
    11      }
    12  
    13      ~this(){
    14          writefln("in dtor");
    15          dtorcalled = true;
    16      }
    17  }
    18  
    19  int main(){
    20      try{
    21          auto MyClass c;
    22          c = new MyClass();
    23      }catch{}
    24  
    25      if(!dtorcalled){
    26          writefln("dtor not called");
    27          return 0;
    28      }else{
    29          writefln("dtor called");
    30          assert(0);
    31      }
    32  
    33      assert(0);
    34  }

Results in:
in ctor
dtor not called
in dtor

So, there seems to be two issues here:
1) the dtor is being called at the wrong place, much after the scope it's declared within has gone away.

2) it's being called at all due to the constructor exiting via throw.

Upgrading to severity major since I can't think of any work around and proper scope behavior is a fairly important language feature.
Comment 2 Walter Bright 2007-10-28 23:27:03 UTC
Works in dmd 1.022 and 2.007.
Comment 3 Gide Nwawudu 2009-01-16 06:05:50 UTC
I noticed this issue in my code. I think the problem is that the following code should not be allowed.

   scope MyClass c; // or auto MyClass c;
   c = new MyClass();

The following code works correctly.
   scope MyClass c = new MyClass();

Maybe the tests should be changed in dstress.
Comment 4 Koroskin Denis 2009-01-16 15:01:17 UTC
I'm not sure whether it is a bug (unless specs specify other behavior).

Unlike C++, objects in D are fully constructed /before/ entering ctor and thus I would expect the dtor to be called even if ctor throws.
Comment 5 Max Samukha 2009-01-17 06:12:26 UTC
Default initializing object's fields doesn't mean full object construction. I do believe that an object should be considered constructed only if its constructor has completed successfully. Otherwise, the constructor should clean after itself and only deallocator (if any), but not destructor, should be run. Currently, deallocator is not run, meaning another bug in the compiler.
Comment 6 Lionello Lunesu 2011-11-19 23:16:05 UTC
(In reply to comment #5)
> Default initializing object's fields doesn't mean full object construction. I
> do believe that an object should be considered constructed only if its
> constructor has completed successfully. Otherwise, the constructor should clean
> after itself and only deallocator (if any), but not destructor, should be run.
> Currently, deallocator is not run, meaning another bug in the compiler.

for what it's worth: C# still calls the finalizer when the ctor throws.
Comment 7 Andrei Alexandrescu 2013-11-15 18:58:06 UTC
On the other hand we don't throw if a struct's ctor throws. So this is an inconsistency.I think it's fair in D to consider a throwing constructor as abandoning all construction of the object.
Comment 8 Denis Shelomovskii 2014-07-11 09:22:43 UTC
---(In reply to Andrei Alexandrescu from comment #7)
> On the other hand we don't throw if a struct's ctor throws. So this is an
> inconsistency.I think it's fair in D to consider a throwing constructor as
> abandoning all construction of the object.

Also `scope` classes behaves like structs:
---
class C
{
    this() { throw new Exception(""); }

    ~this() { assert(0); } // never called
}

void main()
{
    try scope c = new C(); catch(Exception) { }
}
---

As for structs, we also have Issue 13095.
Comment 9 Walter Bright 2014-11-13 10:30:03 UTC
Why was this reopened? I can't figure out what the bug is from the comments.
Comment 10 Steven Schveighoffer 2014-11-13 14:11:10 UTC
(In reply to Gide Nwawudu from comment #3)
> I noticed this issue in my code. I think the problem is that the following
> code should not be allowed.
> 
>    scope MyClass c; // or auto MyClass c;
>    c = new MyClass();
> 
> The following code works correctly.
>    scope MyClass c = new MyClass();
> 
> Maybe the tests should be changed in dstress.

This is the comment where it was reopened, if that helps (I'm with you, I think this should be closed).
Comment 11 hsteoh 2018-01-17 21:06:57 UTC
Does this bug only apply for scope classes? I tested the following code on dmd git master:

------
class MyClass {
        this() {
                writeln("ctor");
                throw new Exception("");
        }
        ~this() {
                writeln("dtor");
        }
}
void main() {
        try {
                scope c = new MyClass;
        } catch(Exception e) {
                // ignore
        }
}
------

Only the ctor is called, not the dtor, because the ctor throws and does not properly construct the object. This is correct behaviour.

However, changing `scope` to `auto` will cause the dtor to still be called, in spite of the ctor throwing an exception.  According to Andrei's comment, this is wrong, since the object was not fully initialized, and the exception from the ctor should be taken to mean that construction of the object has been abandoned. Not sure if this belongs to this bug, though, or to a different bug, since the original complaint involved "auto" classes, which I'm assuming was the original name of scope classes.
Comment 12 anonymous4 2018-01-18 16:32:11 UTC
Isn't it only true for C++ that destructor can't be called on an object that wasn't constructed? In C++ destructor can't be called because there's no default initialization before constructor, but in D I think destructor can be called because it won't run on garbage.
Comment 13 Steven Schveighoffer 2018-01-18 17:06:33 UTC
(In reply to anonymous4 from comment #12)
> Isn't it only true for C++ that destructor can't be called on an object that
> wasn't constructed? In C++ destructor can't be called because there's no
> default initialization before constructor, but in D I think destructor can
> be called because it won't run on garbage.

It's not garbage, but it's also potentially not a valid object (what the dtor is expecting).

I'm unsure who is responsible for cleaning up the non-GC resources. Normally the destructor does this, but if you are throwing in the constructor, then you could do it there. However, base constructors will not be able to catch this condition.

If I had to define it myself, I think it shouldn't be left to the GC to clean it up. If the destructor should be called after the constructor fails via exception, it should happen immediately (and only the destructors for which the constructors have completed).

e.g.:

class A
{
    int x;
    this() { writeln("A.ctor"); x = 5; }
    ~this() { writeln("A.dtor"); assert(x == 5); x = 0; }
}

class B : A
{
    int y;
    this(bool bad) { writeln("B.ctor"); super(); if(bad) throw new Exception(); y = 6; }
    ~this() { writeln("B.dtor"); assert(y == 6); y = 0; }
}

void main()
{
try { new B(true); } catch() {}
writeln("done main");
}

expected output:
A.ctor
B.ctor
A.dtor
done main
Comment 14 anonymous4 2018-01-19 07:57:12 UTC
(In reply to Steven Schveighoffer from comment #13)
> (what the dtor is expecting)

D has options there unlike C++.
Comment 15 Mathias LANG 2019-05-21 01:08:32 UTC
The dtor of scope allocated classes is not called anymore.
Comment 16 feklushkin.denis 2021-12-23 16:15:02 UTC
(I spent a few days looking for an issue caused by this mandatory call of dtor when ctor thrown an exception.)

Why it is need to call dtor if ctor isn't sucessful finished?
For purposes of simple correct cleanup we already have scope guards - it can be used inside of ctor.

And this behaviour should be at least described in the manual.