Convention to separate releasing resources and invalidating memory

[marler8997]

When defining a struct that contains fields pointing to other structures/resources, the developer has to decide whether they want to include "memory invalidation" (setting memory to undefined) inside their deinit function. For example:

const S = struct {
    a: Foo,
    b: Bar,
    pub fn deinit(self: *@This()) void {
        a.deinit();
        b.deinit();
        self.* = undefined;
    }
};

It seems like a reasonable choice to invalidate the memory of the struct to help catch bugs elsewhere in the code that could mistakenly try to use the structure after calling deinit. However, there are cases where the caller may not want to invalidate the memory or may not want to right away.

In the example above, imagine that both Foo and Bar also made the decision to invalidate their memory inside their deinit functions. This would mean that the memory for fields a and b will both be "invalidated" twice.

Some data structures within the standard library solve this problem by providing an additional function alongside deinit that releases resources without invalidation. For example, HashMap contains a deallocate function which only releases its resources. Then its deinit function just calls deallocate then invalidates its memory. This pattern allows the caller to release resources and choose when/where to invalidate the memory if at all.

There are other structures within the standard library that could benefit from this pattern (i.e. ArrayList and friends). However, having a standardized method name to release without invalidation seems beneficial to the users that want to invoke the same semantics through the standard library. For this I propose a standardized name for a member function that releases resources without invalidation. Maybe something like deinitNoInvalidate? Having a longer name is a flag to the user that it's up to them to perform invalidation, and that it's the same as deinit except without the invalidation step.

Given a solution like this, we can properly implement our original struct like this:

const S = struct {
    a: Foo,
    b: Bar,
    pub fn deinit(self: *@This()) void {
        self.deinitNoInvalidate();
        self.* = undefined;
    }
    pub fn deinitNoInvalidate(self: @This()) void {
        a.deinitNoInvalidate();
        b.deinitNoInvalidate();
    }
};

Now our struct only invalidates memory once, and other users who use our struct as a field will be able to do the same.

Another example of excluding memory invalidation inside deinit is when an application wants to create a const instance of a struct. For example, say you create a file handle wrapper type. Assuming the handle is the only data inside the struct, it's reasonable for an application to create a const version of it since the handle should never change. However, forcing memory invalidation just after releasing the handle would prevent an application from using that function.

Variation: don't invalidate in deinit

Another solution is to just say that deinit should NEVER invalidate its own memory. Consequently this could be enforced by requiring that deinit always take self as a value rather than a pointer. Then the decision to invalidate is left to the caller. With this variation the standard lib could define a function that always does both:

pub fn safeDefinit(v: var) { v.deinit(); v.* = undefined }

//
var s = Something.init(...);
defer std.safeDeinit(&s);

One benefit of this variation is types don't have to include boilerplate to define their own function that both releases and invalidates.

[Mouvedia]

This pattern allows the caller to release resources and choose when/where to invalidate the memory if at all.

For the name what about release?

[marler8997]

release definitely looks nicer, I would be fine with either, I think both have pros and cons.

[andrewrk]

In the example above, imagine that both Foo and Bar also made the decision to invalidate their memory inside their deinit functions. This would mean that the memory for fields a and b will both be "invalidated" twice.

That's normal, and OK. This is a perfectly valid, well defined program:

var x: i32 = 1234;
x = undefined;
x = undefined;

Note that setting something to undefined is a no-op in release-fast builds. In release-safe, the second time x is set to undefined is a no-op.

For example, HashMap

HashMap is a good example - it has clearRetainingCapacity and clearAndFree. ArrayList has shrink and shrinkRetainingCapacity.

Given a solution like this, we can properly implement our original struct like this:

I think that code is actually worse, because it's likely that deinitialization is cheaper clearing and maintaining data integrity. The original code is better: ask both to be deinitialized, then set self to undefined.

For the name what about release?

I suggest clearAndFree and clearRetainingCapacity to match HashMap.

Variation: don't invalidate in deinit

This looks like it might be useful as a separate proposal

[jumpnbrownweasel]

Another example of excluding memory invalidation inside deinit is when an application wants to create a const instance of a struct. For example, say you create a file handle wrapper type. Assuming the handle is the only data inside the struct, it's reasonable for an application to create a const version of it since the handle should never change. However, forcing memory invalidation just after releasing the handle would prevent an application from using that function.

To me it makes more sense for init to return a non-const instance and do the invalidation in deinit. Since the instance is closing the file at deinit, it's not logically const anyway. It shouldn't be used after deinit (because the file is closed) so invalidating makes sense.

[andrewrk]

I don't think this proposal has anything actionable. There is always the need for deinit which sets the state back to uninitialized. This convention is common and it makes sense to set to undefined at the end.

There could also be a useful convention for reset or similar that sets the state back to some concept "empty". For example ArrayList clear function does this.

I don't think deinitNoInvalidate is useful enough to warrant existing. I'm not even sure what would happen if this proposal was accepted, I think it's best to close it and move on.