Bug 21693 – extern(C++) class instance dtors are never called, breaking RAII

The program below never destroys its extern(C++) class instances, breaking RAII. Output since 2.066: Destroying stack D instance. Destroying heap D instance. The destructor should definitely be called for the `scope` instance, as it would be for a struct on the stack. The heap instance's destructor should also be called, OR trying to allocate an instance of an extern(C++) class that has a destructor on the GC heap using `new` should be a compile-time error. ///////////////////////////////////// extern(C++) class C { bool onStack; this(bool onStack) { this.onStack = onStack; } ~this() { writeln("Destroying ", onStack? "stack" : "heap", " C instance."); } } extern(D) class D { bool onStack; this(bool onStack) { this.onStack = onStack; } ~this() { writeln("Destroying ", onStack? "stack" : "heap", " D instance."); } } void main() { scope stackC = new C(true); auto heapC = new C(false); scope stackD = new D(true); auto heapD = new D(false); } ///////////////////////////////////// Related: https://issues.dlang.org/show_bug.cgi?id=21692

Potential solution for `scope` instances: https://github.com/dlang/dmd/pull/12265

@MoonlightSentinel updated dlang/dmd pull request #12265 "Issue 21693 - Lower scoped destruction of extern(C++) class to destroy" mentioning this issue: - Issue 21693 - Lower scoped destruction of extern(C++) class to destroy Ensures proper RAII behaviour for stack allocated instances but doesn't affect the behaviour for heap allocated instances. The previous rewrite used `delete` which relies on `TypeInfo` and crashed at runtime. Using `object.destroy` circumvents this issue (and is also beneficial due to the deprecation of `delete`). `destroy` can be used instead of `delete` because the instances live on the heap and hence don't need to be deallocated. https://github.com/dlang/dmd/pull/12265

The GC could presumably handle C++ classes like structs, where the TypeInfo_Struct pointer is stored after the struct instance IIRC. For C++ classes, it could store the D TypeInfo_Class pointer after the class instance (but note that class instances have no tail padding, so some padding might be needed for an aligned TypeInfo pointer).

dlang/dmd pull request #12265 "Issue 21693 - Lower scoped destruction of extern(C++) class to destroy/dtor call" was merged into stable: - e28736c522d9c81acef9c66c1f02da4375f08207 by MoonlightSentinel: Issue 21693 - Lower scoped destruction of extern(C++) class to destroy Ensures proper RAII behaviour for stack allocated instances but doesn't affect the behaviour for heap allocated instances. The previous rewrite used `delete` which relies on `TypeInfo` and crashed at runtime. Using `object.destroy` circumvents this issue (and is also beneficial due to the deprecation of `delete`). `destroy` can be used instead of `delete` because the instances live on the heap and hence don't need to be deallocated. - aa2147b836b129e835828a6e6faef363ed3255a8 by MoonlightSentinel: fixup! Issue 21693 - Lower scoped destruction of extern(C++) class to destroy https://github.com/dlang/dmd/pull/12265

(In reply to kinke from comment #3) > The GC could presumably handle C++ classes like structs, where the > TypeInfo_Struct pointer is stored after the struct instance IIRC. For C++ > classes, it could store the D TypeInfo_Class pointer after the class > instance (but note that class instances have no tail padding, so some > padding might be needed for an aligned TypeInfo pointer). That would be fine. The maximum overhead would be less than (size_t.sizeof * 2), which is no big deal if it gets RAII working properly. Is it necessary to store a TypeInfo pointer for every instance, though? Garbage collected instances would all be allocated on the D side, so I would think that the TypeInfo data could just be stored adjacent to D's copy of the vtable.

I'm dropping the priority from "critical" to "major" now that destruction is performed for `scope` instances, since the GC doesn't strictly guarantee deterministic destruction even for D class instances. Thanks, @moonlightsentinel!

@puneet updated dlang/dmd pull request #12302 "Fix issue 19192 - [wrong-code] [crashes] Covariant method interface <…" mentioning this issue: - Issue 21693 - Lower scoped destruction of extern(C++) class to destroy/dtor call (#12265) * Issue 21693 - Lower scoped destruction of extern(C++) class to destroy Ensures proper RAII behaviour for stack allocated instances but doesn't affect the behaviour for heap allocated instances. The previous rewrite used `delete` which relies on `TypeInfo` and crashed at runtime. Using `object.destroy` circumvents this issue (and is also beneficial due to the deprecation of `delete`). `destroy` can be used instead of `delete` because the instances live on the heap and hence don't need to be deallocated. https://github.com/dlang/dmd/pull/12302

@MoonlightSentinel updated dlang/dmd pull request #12402 "Fix 21229 - Accept construction of union member as initialization" mentioning this issue: - Issue 21693 - Lower scoped destruction of extern(C++) class to destroy/dtor call (#12265) * Issue 21693 - Lower scoped destruction of extern(C++) class to destroy Ensures proper RAII behaviour for stack allocated instances but doesn't affect the behaviour for heap allocated instances. The previous rewrite used `delete` which relies on `TypeInfo` and crashed at runtime. Using `object.destroy` circumvents this issue (and is also beneficial due to the deprecation of `delete`). `destroy` can be used instead of `delete` because the instances live on the heap and hence don't need to be deallocated. https://github.com/dlang/dmd/pull/12402

@MoonlightSentinel created dlang/dmd pull request #12408 "Merge stable into master" mentioning this issue: - Issue 21693 - Lower scoped destruction of extern(C++) class to destroy/dtor call (#12265) * Issue 21693 - Lower scoped destruction of extern(C++) class to destroy Ensures proper RAII behaviour for stack allocated instances but doesn't affect the behaviour for heap allocated instances. The previous rewrite used `delete` which relies on `TypeInfo` and crashed at runtime. Using `object.destroy` circumvents this issue (and is also beneficial due to the deprecation of `delete`). `destroy` can be used instead of `delete` because the instances live on the heap and hence don't need to be deallocated. https://github.com/dlang/dmd/pull/12408

dlang/dmd pull request #12408 "Merge stable into master" was merged into master: - 939778e46731edc6c4f726dda7f82aaa209d012e by Florian: Issue 21693 - Lower scoped destruction of extern(C++) class to destroy/dtor call (#12265) * Issue 21693 - Lower scoped destruction of extern(C++) class to destroy Ensures proper RAII behaviour for stack allocated instances but doesn't affect the behaviour for heap allocated instances. The previous rewrite used `delete` which relies on `TypeInfo` and crashed at runtime. Using `object.destroy` circumvents this issue (and is also beneficial due to the deprecation of `delete`). `destroy` can be used instead of `delete` because the instances live on the heap and hence don't need to be deallocated. https://github.com/dlang/dmd/pull/12408

THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/19883 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB