Bug 2834 – Struct Destructors are not called by the GC, but called on explicit delete.

According to the spec: [Struct] "Destructors are called when an object goes out of scope. Their purpose is to free up resources owned by the struct object." And while the heap is always 'in scope', it does delete structs which arguably places them out of scope. Note that an explicit call to delete will run the destructor. At a minimum, the spec should be updated to clearly reflect this limitation. Since memory leaks, open files, etc will probably occur when a struct with a postblit is manually allocated directly on the heap, instead of on the stack or as part of an object, this issue might warrant a compiler warning/error (i.e. an explicit cast is required).

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

From Issue 4442: Example brought up by Sean Kelly in private correspondence: struct S1{ ~this() { writeln("dtor"); } } void main() { auto a = S1(); auto b = new S1(); delete b; auto c = new S1(); c = null; GC.collect(); } "dtor" is only printed twice.

Gc finalization is not deterministic. You can't expect it to be called.

What about structs whose memory are freed by the GC? Would you expect their dtors to be called? They aren't. Try allocating ten million structs in a loop, not one "dtor" line will print.

Fixing this will probably be fairly involved. The GC will have to store a TypeInfo reference for each block that needs to be finalized. The best approach may be to integrate this with precise scanning, since that requires detailed type info too.

For the moment the compiler can show a warning when the code allocates on the heap a struct that has a destructor.

One case where struct destructors are not called, in this situation it seems simpler for the GC to know what destructors to call: import core.memory: GC; import core.stdc.stdio: printf; struct Foo { int x; this(int xx) { this.x = xx; } ~this() { printf("Foo dtor x: %d\n", x); } } void main() { Foo[] a; a.length = 2; a[0].x = 1; a[1].x = 2; // delete a; } (I am not sure, but a type information can be useful in arrays, maybe to fix bug 2095 too.)

So what is the verdict? Should we simply specify that struct destructors are not automatically called except in RAII and remove the struct-in-class special case? BTW, there are other problems (serious IMO): auto ss = new S[10]; ss.length = 5; delete ss; Destructors are not called on the last 5 elements. ---- auto ss = new S[10]; ss ~= ss; delete ss; We have a nasty problem when destructors are called on the appended elements because postblits was not run for them during append. etc Essentially, operations on arrays of structs with postblits/dtors defined are currently unusable.

(In reply to comment #8) > We have a nasty problem when destructors are called on the appended elements > because postblits was not run for them during append. I meant the problem is the destructors called on objects that have not been postblitted.

Updated flags as it affects all platforms and is general language defect.

Maybe we should put a bounty on it.

AdRoll (the company I work for) just put a $1000 bounty on this bug: https://www.bountysource.com/issues/2900969-struct-destructors-are-not-called-by-the-gc-but-called-on-explicit-delete We are really excited about D, and would generally love to see any work on improving D's GC!

I'd strongly argue that we make it so that struct destructors get called when the memory for a struct is freed on the GC heap (though that pretty requires what precise heap scanning requires as Sean points out). However, it should be pointed out that there's no guarantee (and likely never will be a guarantee) that everything on the heap will be collected, which would mean that it will never be guaranteed that all struct destructors will be called when on the heap - just that they would be called in the cases where the struct is collected and freed by the GC. So, while we should be able to make it so that struct destructors on the GC heap get called much more (as opposed to never), it will almost certainly be the case that the non-GC heap (along with some kind of manual memory management or reference-counting) will have to be used if it's _required_ that the destructor be run (though technically, not even that would guarantee it, since the program could still segfault, or an Error could be thrown, or the computer could lose power or some other incident could occur which would make it so that the program terminated prematurely, in which case, the destructors wouldn't be run).

(In reply to Benoit Rostykus from comment #12) > AdRoll (the company I work for) just put a $1000 bounty on this bug: That's quite the bounty.

Would making all the writeln's in http://dpaste.dzfl.pl/fbb4a15cda14 print out be an acceptable solution to this issue? (that is, making heap-allocated structs call their destructors)

(In reply to Orvid King from comment #15) > Would making all the writeln's in http://dpaste.dzfl.pl/fbb4a15cda14 print > out be an acceptable solution to this issue? (that is, making heap-allocated > structs call their destructors) As far as I can see, this would not necessarily test structs contained in other structs/classes/arrays.

https://github.com/D-Programming-Language/druntime/pull/864 It's not feasibly possible to call destructors on heap-allocated arrays of structures, however this PR does implement the calling of destructors for structs allocated directly on the heap.

(In reply to Orvid King from comment #17) > https://github.com/D-Programming-Language/druntime/pull/864 > > It's not feasibly possible to call destructors on heap-allocated arrays of > structures, however this PR does implement the calling of destructors for > structs allocated directly on the heap. As I understand it, it should be possible to call the destructors for structs in arrays if we have the additional type information required for precise heap scanning, but until we add that, we're stuck. But if you can make it so that the destructors get called in other circumstances, that's a great improvement regardless.

We still wouldn't be able to call the destructors in structs for arrays even with precise heap scanning, because we can't know that each value in the array is actually valid. Take for instance an array of File's, it's not valid to call the destructor of File.init, so we could compare the value of an element of an array of structs to it's init value, but who's to say that element was ever initialized in the first place? How would we detect that? I think the best thing to do with the arrays of structs that have destructors is to simply make them illegal, and to present as an alternative, an array of a structure that you have defined and has an inner alias this to the original structure, but rather than having a destructor, has a named method that the user would have to call themselves for each element that they know is initialized.

(In reply to Orvid King from comment #19) > We still wouldn't be able to call the destructors in structs for arrays even > with precise heap scanning, because we can't know that each value in the > array > is actually valid. Take for instance an array of File's, it's not valid to > call > the destructor of File.init, so we could compare the value of an element of > an > array of structs to it's init value, but who's to say that element was ever > initialized in the first place? How would we detect that? It's perfectly valid to call destructor on T.init. In fact compiler will always do so with stack-allocated instances.

(In reply to Orvid King from comment #19) > We still wouldn't be able to call the destructors in structs for arrays even > with precise heap scanning [...] > How would we detect that? The array handling in lifetime.d should supply a type info with a destructor that iterates over the elements and calls their destructor. You might have to generate that typeinfo at runtime, though.

Ok, so, to give an update on this issue, I've now implemented the invoking of destructors on heap allocated structs, as well as arrays of structs. It required a fix to a bug in DMD with the delete operator, as well as a change (which I still need to make), to the test suite so that it doesn't allocate in a destructor. The DMD PR is here: https://github.com/D-Programming-Language/dmd/pull/3727 And the druntime PR, as mentioned farther up, is here: https://github.com/D-Programming-Language/druntime/pull/864

And another update. The fix to the other issue that this encountered has already been merged, and the main DRuntime PR has been running green on the autotester for a while now. It's just awaiting a final review and merge. I've also been doing my local projects with the man DRuntime PR included, and have yet to encounter any issues on win32, so I believe that it is stable.

Ready to take the big step with 2.067?

Fine by me.

Let's do it. This issue has been waiting since 2009. It's about time we did this right.

Could you please add a function to druntime callStructDtorsDuringGC(bool)? That would give a chance people who have issues with the called destructors to revert back to the old behavior until they fix their code.

Where should I add it? It can't be in rt.lifetime, because that's not exposed to the user.

core.memory?

Commit pushed to master at https://github.com/D-Programming-Language/druntime https://github.com/D-Programming-Language/druntime/commit/dbbea7c5bd4d8936dc8cb523b65f771842f2a487 Merge pull request #864 from Orvid/structDestructors Fix Issue 2834 - The GC will now call destructors on heap allocated structs

Commit pushed to 2.067 at https://github.com/D-Programming-Language/druntime https://github.com/D-Programming-Language/druntime/commit/dbbea7c5bd4d8936dc8cb523b65f771842f2a487 Merge pull request #864 from Orvid/structDestructors