Technical Discussion: Memory Deallocation Strategy for Node and its Dependencies

[AbdelStark]

Memory Deallocation Strategy for Node and its Dependencies

Background

In our current implementation of the Node struct, we have dependencies like Mempool, Storage, P2P, and RPC. These components are initialized outside the Node and passed in during its initialization. Currently, the Node struct's deinit function doesn't handle the deallocation of these dependencies.

Current Implementation

pub const Node = struct {
    mempool: *Mempool,
    storage: *Storage,
    p2p: *P2P,
    rpc: *RPC,
    // ... other fields ...

    pub fn deinit(self: *Node) void {
        _ = self;
    }

    // ... other methods ...
};

fn runNodeCommand(program: *Program) !void {
    // ... initialization of components ...
    var node = try Node.init(&mempool, &storage, &p2p, &rpc);
    defer node.deinit();
    // ... start the node ...
}

The Question

Should the Node struct be responsible for deallocating its dependencies (Mempool, Storage, P2P, RPC) in its deinit function?

Considerations

1. Ownership and Responsibility:

   • Pro: The Node could be seen as the owner of these components, making it logical for it to handle their cleanup.
   • Con: This could lead to confusion if other structs (like a potential Miner) also use these components.

2. Lifetime Management:

   • Pro: Centralizing deallocation in Node.deinit() could simplify lifetime management.
   • Con: It might complicate scenarios where components need to outlive the Node.

3. Flexibility and Reusability:

   • Pro: Keeping deallocation separate allows for more flexible use of components across different structs.
   • Con: It requires careful tracking of who is responsible for cleanup.

4. Thread Safety and Race Conditions:

   • How would centralized deallocation in Node impact thread safety, especially if components are shared?

5. Error Handling:

   • How should errors during deallocation be handled if Node is responsible?

6. Testing and Mocking:

   • How would this decision impact our ability to test Node and mock its dependencies?

7. Zig Best Practices:

   • Are there established patterns in the Zig community for handling this kind of situation?

8. Performance Considerations:

   • Are there any performance implications to consider with either approach?

Possible Approaches

1. Status Quo: Keep deallocation separate, as it is now.
2. Node-Managed Deallocation: Modify Node.deinit() to deallocate all dependencies.
3. Partial Responsibility: Node deallocates some components but not others.
4. Deallocation Callbacks: Pass deallocation functions to Node during initialization.

Questions for Discussion

1. What are the pros and cons of each approach in the context of our project?
2. How does each approach align with Zig's philosophy and best practices?
3. Are there any other approaches we should consider?
4. How would our choice impact future development and maintainability?

Next Steps

• Gather feedback from the community on the proposed approaches.
• Consider implementing a proof of concept for the most promising approach(es).
• Decide on a strategy and document the reasoning for future reference.

Please share your thoughts, experiences, and any additional considerations we should take into account. Your input will help us make an informed decision that balances safety, performance, and maintainability.

[Oppen]

I believe the core question here is who owns what.
If you don't have exclusive ownership of a resource then it's not up to you to decide to destroy it.
If you do, then anyone who wishes to use it needs to ask you for permission to use it to avoid races, and you are also in charge of freeing it.
If it is in fact a shared resource, either you keep status quo or add reference counting, but in no case can you simply free it because you'll have use-after-free errors.
All that said, using callbacks could be appropriate to make an ad-hoc choice. Pass a callback to a nop if you are not allowed to free it, a callback to deinit if you are, and a callback that implements reference counting otherwise.
It seems like another common pattern is to use defer if and store ownership information as flags.
In terms of performance considerations, reference counting has a little bit of overhead and indirect calls may inhibit some optimizations, but unless a metric confirms there is a problem I wouldn't worry of the performance of a deinit function.
On the thread safety front, my suspicion is that either we already work with mutual exclusion or we have a bigger problem than deallocation.

TL;DR: I think the most sensible approach for this is to just use reference counting and make every user increase and decrease the count as needed (by calling a function made to that end). You would also need some kind of mutex, that may be the most costly part.