On 23/05/2018 1:59 PM, sarn wrote:
(I'm referring to Scott's 2014 DConf talk:
https://www.youtube.com/watch?v=KAWA1DuvCnQ)
I was actually preparing a DIP about this when Manu posted to the forums
about his own related problems with C++ interop.
I traced a bug in some of my D code to my own misunderstanding of how
D's destructors actually work. So I did some research and discovered a
bunch of edge cases with using __dtor, __xdtor and
hasElaborateDestructor. I tried reviewing the packages on
code.dlang.org and some elsewhere (thankfully only about 1% of D
packages use these functions directly) and it turns out I'm not the only
one confused. I don't have time to file bug reports for every package,
so, if you're responsible for code that handles destructors manually,
please do a review. There's a *lot* of buggy code out there.
I'm starting this thread to talk about ways to make things better, but
first the bad news. Let's use this idiomatic code as an example of
typical bugs:
void foo(T)(auto ref T t)
{
...
static if (hasElaborateDestructor!T)
{
t.__dtor();
}
...
}
Gotcha #1:
__dtor calls the destructor defined by T, but not the destructor defined
by any members of T (if T is a struct or class). I know, some of you
might be thinking, "Silly sarn, that's what __xdtor is for, of course!"
Turns out this isn't that widely known or understood (__dtor is used in
examples in the spec ---
https://dlang.org/spec/struct.html#assign-overload). A lot of code is
only __dtor-aware, and there's at least some code out there that checks
for both __dtor and __xdtor and mistakenly prefers __dtor. __xdtor only
solves the specific problem of also destroying members.
Gotcha #2:
The destructor will never be run for classes because
hasElaborateDestructor is only ever true for structs. This is actually
per the documentation, but it's also not well known "on the ground"
(e.g., a lot of code has meaningless is(T == class) or is(T == struct)
around hasElaborateDestructor). The code example is obviously a leak if
t was emplace()d in non-GC memory, but even for GCed classes, it's
important for containers to be explicit about whether or not they own
reference types.
Gotcha #3:
Even if __dtor is run on a class instance, it generally won't run the
correct destructor because it's not virtual. (I.e., if T is a base
class, no destructors for derived classes will be called.) The spec
says that D destructors are virtual, but this is emulated using runtime
type information rather than by using the normal virtual function
implementation. __xdtor is the same.
Gotcha #4:
Even if the right derived class __dtor is run, it won't run the
destructors for any base classes. The spec says that destructors
automatically recurse to base classes, but, again, this is handled
manually by walking RTTI, not by making the destructor function itself
recurse.
Gotcha #5:
The idiom of checking if something needs destruction before destroying
it is often implemented incorrectly. As before, hasElaborateDestructor
works for structs, but doesn't always work as expected for classes.
hasMember!(T, "__dtor") seems to work for classes, but doesn't work for
a struct that doesn't define a destructor, but requires destruction for
its members (a case that's easy to miss in testing).
It looks like most D programmers think that D destructors work like they
typically do in C++, just like I did.
Here are some recommendations:
* Really try to just use destroy(). Manually working with
__dtor/__xdtor is a minefield, and I haven't seen any code that actually
reimplements the RTTI walk that the runtime library does.
(Unfortunately destroy() currently isn't zero-overhead for plain old
data structs because it's based on RTTI, but at least it works.)
* Avoid trying to figure out if something needs destruction. Just
destroy everything, or make it clear you don't own classes at all, or be
totally sure you're working with plain old data structs.
* Some code uses __dtor as a way to manually run cleanup code on an
object that will be used again. Putting this cleanup code into a normal
method will cause fewer headaches.
The one other usage of these low-level destructor facilities is checking
if a type is a plain old data struct. This is an important special case
for some code, but everyone seems to do the check a different way.
Maybe a special isPod trait is needed.
I would consider the current state with classes a bug.
So ticket please, it should not require a DIP to change (although Walter
may disagree).
