Background
Currently, all std.hash_map and std.array_hash_map variants implement pointer stability via a pointer_stability: std.debug.SafetyLock field to catch unwanted pointer modification in safe build modes. The same mechanism will likely be used in the future for std.ArrayList* (Issue #19326, PR #22992) and std.MultiArrayList (Issue #19327).
The implementation will probably always look the same:
- Add a
pointer_stabilityfield to the container - Add this to every function that might modify allocations:
self.pointer_stability.lock();
defer self.pointer_stability.unlock();
- Profit
Proposal
Why not move this functionality to an Allocator? The implementation would be very straightforward, basically just an allocator that calls pointer_stability.lock() and pointer_stability.unlock() before/after every alloc/remap/free:
std.debug.LockableAllocator:
child_allocator: std.mem.Allocator,
pointer_stability: std.debug.SafetyLock = .{},
const Self = @This();
fn alloc(...) ?[*]u8 {
const self: *Self = @ptrCast(@alignCast(ptr));
self.pointer_stability.lock();
defer self.pointer_stability.unlock();
return self.child_allocator.rawAlloc(...);
}
fn resize(...) bool {
const self: *Self = @ptrCast(@alignCast(ptr));
return self.child_allocator.rawResize(...);
}
fn remap(...) ?[*]u8 {
const self: *Self = @ptrCast(@alignCast(ptr));
self.pointer_stability.lock();
defer self.pointer_stability.unlock();
return self.child_allocator.rawRemap(...);
}
fn free(...) void {
const self: *Self = @ptrCast(@alignCast(ptr));
self.pointer_stability.lock();
defer self.pointer_stability.unlock();
return self.child_allocator.rawFree(...);
}
This would make it very easy to add pointer stability checks to existing container types, including ones from external dependencies.
Considerations
There are two main problems I see with this:
- Pointer stability assertions in
movefunctions - Accessing the
pointer_stabilityfield from functions that take the protected container as a parameter
About the first problem:
Right now there is a self.pointer_stability.assertUnlocked() in the hash map move() functions. I would argue that this is misplaced anyway, since the actual memory doesn’t get changed in these functions, it’s just the owner of the memory that changes. So I don’t really see the point in asserting pointer stability here, please correct me if I’m wrong.
The only other use of .assertUnlocked() is in the deinit() functions which free memory anyways, so locking + unlocking the SafetyLock in free provides an even stronger guarantee here.
About the second problem:
This is obviously a bit of an inconveniance. Currently you can just pass a *std.HashMapUnmanaged to any function and then access .pointer_stability directly to check whether it’s (un)locked. With this proposal you would either have to do this explicitly as a separate parameter or build a wrapper around the protected container that contains a reference to the pointer stability lock.
But I think it’s still worth it for the added flexibility and ease of implementation. Also not every function needs to check for pointer stability. What if you just want to look up whether a hash map contains an entry? In that case you don’t care about where this entry is stored anyways.
IMO this change would promote explicitness and make the programmer think even harder about whether a function cares about pointer stability or not 
I just thought of this while showering and definitely not long/hard enough to open a GitHub issue/PR, please let me know your opinion on the proposal and whether I’m missing any obvious drawbacks to this approach!