Remove manual leak detection in test (#305)

This PR removes the hardcoded gpa, with manual leak detection from the TestState used in tests.

I believe the reason for this was to have more stack trace frames than what the std.testing.allocator offers, but this proved problematic when using the TestingState in other contexts like benchmarking.

See #275 (comment)

src/ledger/insert_shred.zig

@@ -1126,14 +1126,14 @@ const ShredInserterTestState = struct {
     db: BlockstoreDB,
     inserter: ShredInserter,
 
-    pub fn init(comptime test_name: []const u8) !ShredInserterTestState {
-        return initWithLogger(test_name, sig.trace.TestLogger.default.logger());
+    pub fn init(allocator_: std.mem.Allocator, comptime test_name: []const u8) !ShredInserterTestState {
+        return initWithLogger(allocator_, test_name, sig.trace.TestLogger.default.logger());
     }
 
-    fn initWithLogger(comptime test_name: []const u8, logger: sig.trace.Logger) !ShredInserterTestState {
-        const state = try TestState.init(test_name);
+    fn initWithLogger(allocator_: std.mem.Allocator, comptime test_name: []const u8, logger: sig.trace.Logger) !ShredInserterTestState {
+        const state = try TestState.init(allocator_, test_name);
         const inserter = try ShredInserter.init(
-            state.allocator(),
+            state.allocator,
             logger,
             &state.registry,
             state.db,
@@ -1142,7 +1142,7 @@ const ShredInserterTestState = struct {
     }
 
     pub fn allocator(self: ShredInserterTestState) Allocator {
-        return self.state.allocator();
+        return self.state.allocator;
     }
 
     /// Test helper to convert raw bytes into shreds and pass them to insertShreds
@@ -1199,7 +1199,7 @@ pub fn insertShredsForTest(
 }
 
 test "insertShreds single shred" {
-    var state = try ShredInserterTestState.init("insertShreds single shred");
+    var state = try ShredInserterTestState.init(std.testing.allocator, "insertShreds single shred");
     defer state.deinit();
     const allocator = std.testing.allocator;
     const shred = try Shred.fromPayload(allocator, &sig.ledger.shred.test_data_shred);
@@ -1214,7 +1214,7 @@ test "insertShreds single shred" {
 }
 
 test "insertShreds 100 shreds from mainnet" {
-    var state = try ShredInserterTestState.init("insertShreds 32 shreds");
+    var state = try ShredInserterTestState.init(std.testing.allocator, "insertShreds 32 shreds");
     defer state.deinit();
 
     const shred_bytes = test_shreds.mainnet_shreds;
@@ -1239,7 +1239,7 @@ test "insertShreds 100 shreds from mainnet" {
 
 // agave: test_handle_chaining_basic
 test "chaining basic" {
-    var state = try ShredInserterTestState.init("handle chaining basic");
+    var state = try ShredInserterTestState.init(std.testing.allocator, "handle chaining basic");
     defer state.deinit();
 
     const shreds = test_shreds.handle_chaining_basic_shreds;
@@ -1312,7 +1312,7 @@ test "chaining basic" {
 
 // agave: test_merkle_root_metas_coding
 test "merkle root metas coding" {
-    var state = try ShredInserterTestState.initWithLogger("handle chaining basic", .noop);
+    var state = try ShredInserterTestState.initWithLogger(std.testing.allocator, "handle chaining basic", .noop);
     defer state.deinit();
     const allocator = state.allocator();
 
@@ -1445,7 +1445,7 @@ test "merkle root metas coding" {
 
 // agave: test_recovery
 test "recovery" {
-    var state = try ShredInserterTestState.init("handle chaining basic");
+    var state = try ShredInserterTestState.init(std.testing.allocator, "handle chaining basic");
     defer state.deinit();
     const allocator = state.allocator();
 

src/ledger/tests.zig

@@ -50,9 +50,9 @@ test "put/get data consistency for merkle root" {
 
 // Analogous to [test_get_rooted_block](https://github.com/anza-xyz/agave/blob/a72f981370c3f566fc1becf024f3178da041547a/ledger/src/blockstore.rs#L8271)
 test "insert shreds and transaction statuses then get blocks" {
-    var state = try State.init("insert shreds and transaction statuses then get blocks");
+    var state = try State.init(std.testing.allocator, "insert shreds and transaction statuses then get blocks");
     defer state.deinit();
-    const allocator = state.allocator();
+    const allocator = state.allocator;
 
     var db = state.db;
     var inserter = try state.shredInserter();
@@ -349,42 +349,29 @@ pub fn TestState(scope: []const u8) type {
         registry: sig.prometheus.Registry(.{}),
         lowest_cleanup_slot: sig.sync.RwMux(Slot),
         max_root: std.atomic.Value(Slot),
-
-        // if this leaks, you forgot to call `TestState.deinit`
-        _leak_check: *u8,
-
-        /// This is used instead of std.testing.allocator because it includes more stack trace frames
-        /// std.testing.allocator is already the same exact allocator, just with a call to detectLeaks
-        /// run at the end of the test. TestState does the same, so we can use the gpa directly.
-        var gpa = std.heap.GeneralPurposeAllocator(.{ .stack_trace_frames = 100 }){};
-        /// This is private to ensure _leak_check is initialized before this is used.
-        const _allocator = gpa.allocator();
+        allocator: std.mem.Allocator,
 
         const Self = @This();
 
-        pub fn init(comptime test_name: []const u8) !*Self {
-            const self = try _allocator.create(Self);
+        pub fn init(allocator: std.mem.Allocator, comptime test_name: []const u8) !*Self {
+            const self = try allocator.create(Self);
             self.* = .{
-                .db = try TestDB(scope).initCustom(_allocator, test_name),
-                .registry = sig.prometheus.Registry(.{}).init(_allocator),
+                .allocator = allocator,
+                .db = try TestDB(scope).initCustom(allocator, test_name),
+                .registry = sig.prometheus.Registry(.{}).init(allocator),
                 .lowest_cleanup_slot = sig.sync.RwMux(Slot).init(0),
                 .max_root = std.atomic.Value(Slot).init(0),
-                ._leak_check = try std.testing.allocator.create(u8),
             };
             return self;
         }
 
-        pub fn allocator(_: Self) Allocator {
-            return _allocator;
-        }
-
         pub fn shredInserter(self: *Self) !ledger.ShredInserter {
-            return ledger.ShredInserter.init(_allocator, test_logger, &self.registry, self.db);
+            return ledger.ShredInserter.init(self.allocator, test_logger, &self.registry, self.db);
         }
 
         pub fn writer(self: *Self) !ledger.BlockstoreWriter {
             return try ledger.BlockstoreWriter.init(
-                _allocator,
+                self.allocator,
                 test_logger,
                 self.db,
                 &self.registry,
@@ -395,7 +382,7 @@ pub fn TestState(scope: []const u8) type {
 
         pub fn reader(self: *Self) !ledger.BlockstoreReader {
             return try ledger.BlockstoreReader.init(
-                _allocator,
+                self.allocator,
                 test_logger,
                 self.db,
                 &self.registry,
@@ -407,9 +394,7 @@ pub fn TestState(scope: []const u8) type {
         pub fn deinit(self: *Self) void {
             self.db.deinit();
             self.registry.deinit();
-            std.testing.allocator.destroy(self._leak_check);
-            _allocator.destroy(self);
-            _ = gpa.detectLeaks();
+            self.allocator.destroy(self);
         }
     };
 }