Bug 1164 – Unordered GC finalization leading to memory bugs

class Storage { void sync() {}; } class Test { Storage s; this() {s=new Storage;} ~this() {s.sync();} } void main() { new Test; } Code above triggers Segmentation fault. Didn't test it on Windows, but probably it is not platform specific issue.

Invalid. http://www.digitalmars.com/d/class.html#destructors has the following passage: "When the garbage collector calls a destructor for an object of a class that has members that are references to garbage collected objects, those references are no longer valid. This means that destructors cannot reference sub objects. This rule does not apply to auto objects or objects deleted with the DeleteExpression."

[email protected] wrote: > > "When the garbage collector calls a destructor for an object of a class > that has members that are references to garbage collected objects, those > references are no longer valid. This means that destructors cannot > reference sub objects. This rule does not apply to auto objects or objects > deleted with the DeleteExpression." > Hmmm... but it doesn't help much with cleaning up on destruction. When collecting would be done in two phases (1. calling all destructors, 2. deallocating memory) maybe it would work. Maybe I should mark it as enhancement?

Reopened and marked as enhancement.

It also fails in Windows. The workaround is to use the 'scope' attribute. void main() { scope Test t = new Test; }

Reproduced with DMD version 2.065. If the destructor cannot reference sub objects, this implies that also an invariant cannot reference sub objects, because an invariant is called just before the destructor. Example below triggers segfault with DMD version 2.065. class B { double a, b, c, d, e, f, g, h; bool fun() const { return true; } } class A { B b; invariant() { assert(b !is null); if (b.fun()) // <- Segfault here, but b is not null. { assert(true); } } this() { b = new B(); } ~this() {} } int main() { A a = new A(); return 0; }

Couldn't reproduce on dmd 2.072.1. Any better code sample?

The following code still segfaults with DMD 2.072.1-0. class X { double a, b, c, d, e, f, g, h; X sibling; bool fun() const { return true; } invariant() { if(sibling !is null) { if (sibling.fun()) { assert(true); } } } this() {sibling = new X(this); } this(X sibling_) {sibling = sibling_;} ~this() {} } int main() { X x = new X(); return 0; } Garbage collector + destructor (or finalizer) is a difficult combination. See for instance this comment (https://github.com/WebAssembly/design/issues/238#issuecomment-116877193) that strongly opposes introducing a destructor in Javascript because this combination can lead to object resurrection (objects made reachable again in a destructor call).

Cool, was able to repro. Fortunately we have a couple of cards in our sleeve. What we could do is: * Mark * For each collectable object: * Call dtor * (NEW) Obliterate with .init * Sweep That way objects used during destruction will at least find objects in a deterministic state.

Just to be sure I understand you correctly. Am I right that the garbage collector currently works as follows: * Mark (= mark all reachable objects as reachable) * For each collectable (= non-reachable) object: * Call dtor * release memory of the collectable object Am I right that the steps you are thinking about can also be formulated as follows: * Mark (= mark all reachable objects as reachable) * For each collectable (= non-reachable) object: * Call dtor * Obliterate with .init * For each collectable object: * release memory of the collectable object

(In reply to Pieter Penninckx from comment #9) > Just to be sure I understand you correctly. > > Am I right that the garbage collector currently works as follows: > > * Mark (= mark all reachable objects as reachable) > * For each collectable (= non-reachable) object: > * Call dtor > * release memory of the collectable object I don't know exactly. I am pretty certain, however, that freed objects are currently not overwritten with .init.

(In reply to Pieter Penninckx from comment #7) > > int main() { > X x = new X(); > return 0; > } Your invariant gets called infinitely: x.sibling.sibling == x sibling's invariant() executes before and after sibling.fun() executes. The invariant has the line: if (sibling.fun())

(In reply to Pieter Penninckx from comment #7) > > invariant() { > if(sibling !is null) { > if (sibling.fun()) > { assert(true); } > } > } > Also: https://dlang.org/spec/contracts.html#Invariants "The code in the invariant may not call any public non-static members of the class or struct, either directly or indirectly. Doing so will result in a stack overflow, as the invariant will wind up being called in an infinitely recursive manner."

(In reply to Pieter Penninckx from comment #9) > > Am I right that the garbage collector currently works as follows: It currently works as follows: * Mark * For each unmarked object: * Finalize it if necessary * If it can be released without overwriting it do so * For each unmarked unreleased object: * release memory of the object However, you shouldn't rely on this: http://dlang.org/spec/class.html#destructors If you recompile druntime with the MEMSTOMP option, the GC and it will write arbitrary data to finalized memory. Therefore it follows that referencing GC managed objects from invariants of other GC managed objects is unadvised. I think a case could be made for being able to control insertion of invariant calls in destructors.

(In reply to comment #11) Woops. You're right. The segfault is caused by an infinite recursion and probably not by a GC problem. We can avoid the infinite recursion by marking fun() as private: class X { X sibling; private bool fun() const { return true; } // Note: this line changed. invariant() { if(sibling !is null) { if (sibling.fun()) { assert(true); } } } this() {sibling = new X(this); } this(X sibling_) {sibling = sibling_;} ~this() {} } int main() { X x = new X(); return 0; } This just runs normally for me. But if I understand comment #13 correctly, this is just luck and I shouldn't count on it, especially when the runtime is compiled with the MEMSTOMP option.

(In reply to Pieter Penninckx from comment #14) > > This just runs normally for me. But if I understand comment #13 correctly, > this is just luck and I shouldn't count on it, especially when the runtime > is compiled with the MEMSTOMP option. MEMSTOMP is essentially a tool used for debugging memory corruption type issues. Compliant D code runs correctly regardless whether it is enable and I was using it as an illustration of what is allowed in the runtime. I don't know what is the official stance on referencing other GC managed memory from invariant code. Since it is automatically run before the destructor/finalizer, it violates the spec. This restriction is quite severe, it might be better if invariants aren't called before the destructor.

We advertise everywhere that you should not access GC-allocated items in destructors, but you can access anything that is allocated with another method. Just like you should not allocate in destructors. Closing this, as it is not actionable. Anything addressing this should be a DIP, as we have gathered over a decade of experience with this problem.