D's Destructors are What Scott Meyers Warned Us About

[sarn]

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