Bug 704 – `class` destructor is called even if constructor throws

(originally posted by Sean Kelly <[email protected]> on 2006-01-24 as news:[email protected]) // 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

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.

Works in dmd 1.022 and 2.007.

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.

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.

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.

(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.

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.

---(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.

Why was this reopened? I can't figure out what the bug is from the comments.

(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).

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.

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.

(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

(In reply to Steven Schveighoffer from comment #13) > (what the dtor is expecting) D has options there unlike C++.

The dtor of scope allocated classes is not called anymore.

(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.