Bug 5667 – [GSoC] "clear" does not call destructors on structs embedded in structs

import std.traits; import std.conv; import core.stdc.stdlib; struct S { static int dtorCalled; ~this() { dtorCalled++; } } class C { S s; } struct S2 { S s; } void main() { // 1 enum size = __traits(classInstanceSize, C); auto buf = malloc(size)[0..size]; emplace!C(buf); clear(buf); assert(S.dtorCalled == 1); free(buf.ptr); // 2 S2* s2 = cast(S2*)malloc(S2.sizeof); clear(*s2); assert(S.dtorCalled == 2); free(cast(void*)s2); } While a solution to case 2 can be hacked up, case 1 requires a correct destructor (one that calls destructors on the embedded objects and desirably the base class destructor) and a pointer to that destructor in the classinfo.

Your first case is invalid. Clear depends on the type system: auto buf = malloc(size)[0..size]; emplace!C(buf); clear(buf); // here, typeof(buf) == void[], will not call the class version This does work: auto buf = malloc(size)[0..size]; auto c = emplace!C(buf); clear(c); The second case, I agree it's a bug. Even clearing a stack-based struct does not call the embedded dtor: S2 s2; clear(s2); assert(dtorCalled == 1); // fails

(In reply to comment #1) > This does work: > > auto buf = malloc(size)[0..size]; > auto c = emplace!C(buf); > clear(c); > Damn, of course. Sorry.

I think I found a fix, but I'm not sure if it's correct as I don't know much about the D runtime. In file object_.d, line 2600: void clear(T)(ref T obj) if (is(T == struct)) { - static if (is(typeof(obj.__dtor()))) - { - obj.__dtor(); - } + typeid(T).destroy( &obj ); auto buf = (cast(ubyte*) &obj)[0 .. T.sizeof]; auto init = cast(ubyte[])typeid(T).init(); if(init.ptr is null) // null ptr means initialize to 0s buf[] = 0; else buf[] = init[]; } This fixes it for me. You need to modifiy import/object.di as well, because object_.d doesn't get header'd into object.di, for some reason I don't know. I might make a pull request, but I'm not 100% this is correct, I would like to get some feedback.

The code for TypeInfo_Struct.destroy is in object_.d: https://github.com/D-Programming-Language/druntime/blob/master/src/object_.d#L986 It appears that it's using quite a different method to destroy the type (somewhat necessary since TypeInfo is runtime information). I'm curious why calling __dtor isn't the same... That probably should be the focus of this bug instead of working around the issue. (In reply to comment #3) > This fixes it for me. You need to modifiy import/object.di as well, because > object_.d doesn't get header'd into object.di, for some reason I don't know. object.di is hand-maintained to remove some private things that are not necessary to import. The less things defined in the .di, the less internal implementation is exposed, and the less chance someone can exploit implementation details that shouldn't be relied upon. I believe it is the only hand-maintained interface file in druntime. For templates, however, there is no possibility of implementation hiding -- the entire source must be available. I wonder if the templates of object_.d could be separated into another module so we don't have to modify two files...

*** Issue 6203 has been marked as a duplicate of this issue. ***

This is similar issue of bug5661. T.__dtor == ~this(). So a struct type may not have __dtor. But TypeInfo_Struct.xdtor specifies internal destructor calling function. It calls *all* destructors of members even if itself does not have user-defined destructor. I think using typeof(T).destroy is right way for fixing this bug.

Yes, clearly that is a good path. But my question is, why __dtor not the same as destroy? Is there any reason to call ~this() without calling all the sub-dtors? In my opinion, ~this() should implicitly contain the __dtors for all the members at the end. But yes, having clear use typeid().destroy will fix the original symptom. If we don't intend to fix the root cause, we should at least document that nobody should ever use __dtor...

(In reply to comment #7) > Yes, clearly that is a good path. > > But my question is, why __dtor not the same as destroy? Is there any reason to > call ~this() without calling all the sub-dtors? Basically the name "__dtor" is beginning with double underscores, so it is internal name. At least calling it is not "Right D Way", I think. (This point is different from C++ syntax: T t(); t.~T();) > In my opinion, ~this() should implicitly contain the __dtors for all the > members at the end. https://github.com/D-Programming-Language/dmd/blob/master/src/clone.c#L496 Looking at dmd source, the internal function name for its purpose may be "__fieldDtor" or "__aggrDtor", but we can't call them explicitly. > But yes, having clear use typeid().destroy will fix the original symptom. If > we don't intend to fix the root cause, we should at least document that nobody > should ever use __dtor... Agreed. Today calling __dtor is obviously incorrect idiom. ---- extern(C) int printf(const(char*) fmt, ...); struct X { ~this(){ printf("X.~this()\n"); } } struct S { X x; ~this(){ printf("S.~this()\n"); } } void main() { S s; s.__dtor(); printf("-\n"); } ---- Output: ---- S.~this() // s.__dtor() only call S.~this(), never call s.x.~this() - S.~this() X.~this() ----

Created a pull request with the fix and a unittest. https://github.com/D-Programming-Language/druntime/pull/45

The pull request was merged.