On Wednesday, 27 March 2024 at 21:43:48 UTC, Liam McGillivray wrote:
In my current [game project](https://github.com/LiamM32/Open_Emblem), [something strange](https://github.com/LiamM32/Open_Emblem/issues/20) has happened as of a recent commit. When running `dub test`, all the unittests appear to pass, but then after the last unittest has concluded an "Invalid memory operation" happens. Removing a few lines replaces this error with a segfault, but either way, these errors shouldn't be happening. Weirdly, the commit where these errors first appear did nothing but replace a single class-member function with a seemingly identical function through a mixin template.

The errors seem to be related to object deletion. The traceback output for the first error, and where GDB points to for the second error, is the destructor for my `Unit` class.

You see, every `Unit` object is associated with a particular `Map` object and `Faction` object, which it hold references to. Those objects in turn each have an array of `Unit` objects that they are associated with. In the `Unit` destructor, I have it call the `removeUnit` function in both the associated `Map` and `Faction` objects. The errors are happening in either the `Unit` destructor itself, or the `removeUnit` function that it calls. Until the commit that introduced these errors, the `removeUnit` function was written directly in the `Map` class in it's module, but this commit replaced it with the mixin template `UnitArrayManagement`, which `Faction` also uses.

`Unit` destructor:
```
    ~this() {
        this.alive = false;
        if (this.map !is null) this.map.removeUnit(this);
if (this.faction !is null) this.faction.removeUnit(this); if (this.currentTile !is null) this.currentTile.occupant = null;
    }
```

The GC does not guarantee destructor order. So this code is not valid -- e.g. you can't count on `map` to be a valid object at this point.

In my estimation, the code is not correct in principle anyway -- if the `map` has a pointer to the `unit`, then neither will be collected without both being garbage, and so there is no need to do these calls (they are all going away presently).

The *only* thing you should be doing in a destructor is freeing non-GC resources.

I read that the garbage collector *sometimes* but not *always* calls destructors on deletion, which sounds crazy to me.

The GC is not guaranteed to delete memory or run destructors. In the current implementation, it will destroy everything at the end of the program that was allocated using the GC, but the language does not guarantee this.

The second error, which can be achieved by removing the instances of `writeln` in `removeUnit` (making it seemingly identical now to the version of this function previously defined in the `Map` class) is also strange. It seems to be a case of the `Unit` object calling a `Map` object that no longer exists. However, that's also strange, as the `Map` object is supposed to delete all it's associated units on destruction.

As mentioned, GCs do not work this way -- you do not need to worry about cascading removal of anything.

You should assume in the destructor that all references in the type that were pointing at GC blocks are now invalid (i.e. dangling pointers).


So why are these things even happening *after* the unittests have been run? What else do I need to know about object destruction? What may be happening?

The GC is cleaning up all allocated memory, in *no particular order*.

-Steve

Reply via email to