a73x

52e5cdc1

scenario: plug capture leaks + thread CellGeom + distinct errors

a73x   2026-04-19 09:42

Commit message
scenario: plug capture leaks + thread CellGeom + distinct errors

Critical fixes:
  - Capture branch now OOM-safe: all allocations complete before
    committing to the captures map; errdefers clean up on failure.
  - Duplicate-label capture frees the prior image + reuses old key.
  - last_capture_label UAF closed (dupe before freeing old).

Important fixes:
  - AssertCellWithoutCapture is a distinct error from AssertFailed.
  - CellGeom threads through ScenarioState.init for Plan 3 use.

Minor fix:
  - assert-cell-at bounds-check diagnostic now says the right name.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/scenario.zig
Old New
@@ -434,7 +434,7 @@ pub fn parse(
434 return error.ParseFailed; 434 return error.ParseFailed;
435 }; 435 };
436 if (row >= rows or col >= cols) { 436 if (row >= rows or col >= cols) {
437 diag.* = .{ .line = line_num, .message = "assert-cell row/col out of bounds for scenario size" }; 437 diag.* = .{ .line = line_num, .message = "assert-cell-at row/col out of bounds for scenario size" };
438 return error.ParseFailed; 438 return error.ParseFailed;
439 } 439 }
440 try directives.append(alloc, .{ .assert_cell_at = .{ 440 try directives.append(alloc, .{ .assert_cell_at = .{
@@ -781,6 +781,7 @@ pub const TickError = error{
781 ScenarioTimeout, 781 ScenarioTimeout,
782 SleepUntilFlipTimeout, 782 SleepUntilFlipTimeout,
783 AssertFailed, 783 AssertFailed,
784 AssertCellWithoutCapture,
784 PredicateOnMissingLabel, 785 PredicateOnMissingLabel,
785 CallbackFailed, 786 CallbackFailed,
786 } || std.mem.Allocator.Error; 787 } || std.mem.Allocator.Error;
@@ -799,12 +800,14 @@ pub const ScenarioState = struct {
799 sleep_until_flip_started_ns: ?i128, // set when entering a sleep-until-flip directive 800 sleep_until_flip_started_ns: ?i128, // set when entering a sleep-until-flip directive
800 captures: std.StringHashMapUnmanaged(png.Image), // label → captured PNG 801 captures: std.StringHashMapUnmanaged(png.Image), // label → captured PNG
801 last_capture_label: ?[]const u8, // most recently captured label (owned by alloc) 802 last_capture_label: ?[]const u8, // most recently captured label (owned by alloc)
803 cell_geom: CellGeom, // renderer cell dimensions for assert predicates
802 alloc: std.mem.Allocator, 804 alloc: std.mem.Allocator,
803 805
804 pub fn init( 806 pub fn init(
805 alloc: std.mem.Allocator, 807 alloc: std.mem.Allocator,
806 scenario: *const Scenario, 808 scenario: *const Scenario,
807 origin_ns: i128, 809 origin_ns: i128,
810 cell_geom: CellGeom,
808 ) ScenarioState { 811 ) ScenarioState {
809 return .{ 812 return .{
810 .scenario = scenario, 813 .scenario = scenario,
@@ -815,6 +818,7 @@ pub const ScenarioState = struct {
815 .sleep_until_flip_started_ns = null, 818 .sleep_until_flip_started_ns = null,
816 .captures = .{}, 819 .captures = .{},
817 .last_capture_label = null, 820 .last_capture_label = null,
821 .cell_geom = cell_geom,
818 .alloc = alloc, 822 .alloc = alloc,
819 }; 823 };
820 } 824 }
@@ -883,23 +887,39 @@ pub const ScenarioState = struct {
883 std.log.warn("scenario: capture callback failed: {s}", .{@errorName(err)}); 887 std.log.warn("scenario: capture callback failed: {s}", .{@errorName(err)});
884 return error.CallbackFailed; 888 return error.CallbackFailed;
885 }; 889 };
886 // Dupe the label into ScenarioState.alloc so captures outlive the arena. 890 errdefer self.alloc.free(img.pixels);
887 const label_copy = try self.alloc.dupe(u8, label); 891
888 errdefer self.alloc.free(label_copy); 892 // Dupe label for both the map key and last_capture_label. Do both
889 try self.captures.put(self.alloc, label_copy, img); 893 // allocs BEFORE committing to the map so any OOM unwinds cleanly.
890 // Update last_capture_label: free previous, dupe new from original label. 894 const key_dup = try self.alloc.dupe(u8, label);
891 if (self.last_capture_label) |old| self.alloc.free(old); 895 errdefer self.alloc.free(key_dup);
892 self.last_capture_label = try self.alloc.dupe(u8, label); 896 const new_last = try self.alloc.dupe(u8, label);
897 errdefer self.alloc.free(new_last);
898
899 // Reserve the map slot. getOrPut may OOM; errdefers above handle it.
900 const gop = try self.captures.getOrPut(self.alloc, key_dup);
901
902 // Past here: no more OOM-able operations — commit everything atomically.
903 if (gop.found_existing) {
904 // Free old pixel buffer and our freshly-allocated key_dup;
905 // the map retains the existing key slot.
906 self.alloc.free(gop.value_ptr.pixels);
907 self.alloc.free(key_dup);
908 }
909 gop.value_ptr.* = img;
910
911 if (self.last_capture_label) |old_last| self.alloc.free(old_last);
912 self.last_capture_label = new_last;
913
893 self.cursor += 1; 914 self.cursor += 1;
894 }, 915 },
895 .assert_cell => |ac| { 916 .assert_cell => |ac| {
896 const lbl = self.last_capture_label orelse { 917 const lbl = self.last_capture_label orelse {
897 std.debug.print("scenario: assert-cell with no prior capture\n", .{}); 918 std.debug.print("scenario: assert-cell with no prior capture\n", .{});
898 return error.AssertFailed; 919 return error.AssertCellWithoutCapture;
899 }; 920 };
900 const img = self.captures.get(lbl).?; 921 const img = self.captures.get(lbl).?;
901 const geom = CellGeom{ .cell_w_px = 8, .cell_h_px = 16 }; 922 const result = evalPredicate(img, ac.row, ac.col, self.cell_geom, ac.pred, null);
902 const result = evalPredicate(img, ac.row, ac.col, geom, ac.pred, null);
903 if (!result.pass) { 923 if (!result.pass) {
904 std.debug.print("scenario: assert-cell ({d},{d}) {s} failed: {s}\n", .{ 924 std.debug.print("scenario: assert-cell ({d},{d}) {s} failed: {s}\n", .{
905 ac.row, ac.col, @tagName(ac.pred), result.reason, 925 ac.row, ac.col, @tagName(ac.pred), result.reason,
@@ -912,8 +932,7 @@ pub const ScenarioState = struct {
912 const img = self.captures.get(aca.label) orelse { 932 const img = self.captures.get(aca.label) orelse {
913 return error.PredicateOnMissingLabel; 933 return error.PredicateOnMissingLabel;
914 }; 934 };
915 const geom = CellGeom{ .cell_w_px = 8, .cell_h_px = 16 }; 935 const result = evalPredicate(img, aca.row, aca.col, self.cell_geom, aca.pred, null);
916 const result = evalPredicate(img, aca.row, aca.col, geom, aca.pred, null);
917 if (!result.pass) { 936 if (!result.pass) {
918 std.debug.print("scenario: assert-cell-at {s} ({d},{d}) {s} failed: {s}\n", .{ 937 std.debug.print("scenario: assert-cell-at {s} ({d},{d}) {s} failed: {s}\n", .{
919 aca.label, aca.row, aca.col, @tagName(aca.pred), result.reason, 938 aca.label, aca.row, aca.col, @tagName(aca.pred), result.reason,
@@ -986,7 +1005,7 @@ test "tick: empty scenario is immediately done" {
986 \\timeout 1000ms 1005 \\timeout 1000ms
987 ); 1006 );
988 defer s.deinit(); 1007 defer s.deinit();
989 var state = ScenarioState.init(std.testing.allocator, &s, 0); 1008 var state = ScenarioState.init(std.testing.allocator, &s, 0, .{ .cell_w_px = 8, .cell_h_px = 16 });
990 defer state.deinit(); 1009 defer state.deinit();
991 1010
992 try std.testing.expect(state.isDone()); 1011 try std.testing.expect(state.isDone());
@@ -999,7 +1018,7 @@ test "tick: bytes directive fires write_bytes immediately" {
999 \\bytes "abc" 1018 \\bytes "abc"
1000 ); 1019 );
1001 defer s.deinit(); 1020 defer s.deinit();
1002 var state = ScenarioState.init(std.testing.allocator, &s, 0); 1021 var state = ScenarioState.init(std.testing.allocator, &s, 0, .{ .cell_w_px = 8, .cell_h_px = 16 });
1003 defer state.deinit(); 1022 defer state.deinit();
1004 1023
1005 var tio = TestIO{ .alloc = std.testing.allocator }; 1024 var tio = TestIO{ .alloc = std.testing.allocator };
@@ -1019,7 +1038,7 @@ test "tick: sleep holds advancement until time passes" {
1019 \\bytes "x" 1038 \\bytes "x"
1020 ); 1039 );
1021 defer s.deinit(); 1040 defer s.deinit();
1022 var state = ScenarioState.init(std.testing.allocator, &s, 0); 1041 var state = ScenarioState.init(std.testing.allocator, &s, 0, .{ .cell_w_px = 8, .cell_h_px = 16 });
1023 defer state.deinit(); 1042 defer state.deinit();
1024 1043
1025 var tio = TestIO{ .alloc = std.testing.allocator }; 1044 var tio = TestIO{ .alloc = std.testing.allocator };
@@ -1048,7 +1067,7 @@ test "tick: capture calls io.capture and stores the result under the label" {
1048 \\capture snap1 1067 \\capture snap1
1049 ); 1068 );
1050 defer s.deinit(); 1069 defer s.deinit();
1051 var state = ScenarioState.init(std.testing.allocator, &s, 0); 1070 var state = ScenarioState.init(std.testing.allocator, &s, 0, .{ .cell_w_px = 8, .cell_h_px = 16 });
1052 defer state.deinit(); 1071 defer state.deinit();
1053 1072
1054 var tio = TestIO{ .alloc = std.testing.allocator }; 1073 var tio = TestIO{ .alloc = std.testing.allocator };
@@ -1069,7 +1088,7 @@ test "tick: scenario timeout fires" {
1069 \\bytes "x" 1088 \\bytes "x"
1070 ); 1089 );
1071 defer s.deinit(); 1090 defer s.deinit();
1072 var state = ScenarioState.init(std.testing.allocator, &s, 0); 1091 var state = ScenarioState.init(std.testing.allocator, &s, 0, .{ .cell_w_px = 8, .cell_h_px = 16 });
1073 defer state.deinit(); 1092 defer state.deinit();
1074 1093
1075 var tio = TestIO{ .alloc = std.testing.allocator }; 1094 var tio = TestIO{ .alloc = std.testing.allocator };
@@ -1088,7 +1107,7 @@ test "tick: sleep-until-flip holds until flip callback returns true" {
1088 \\bytes "x" 1107 \\bytes "x"
1089 ); 1108 );
1090 defer s.deinit(); 1109 defer s.deinit();
1091 var state = ScenarioState.init(std.testing.allocator, &s, 0); 1110 var state = ScenarioState.init(std.testing.allocator, &s, 0, .{ .cell_w_px = 8, .cell_h_px = 16 });
1092 defer state.deinit(); 1111 defer state.deinit();
1093 1112
1094 var tio = TestIO{ .alloc = std.testing.allocator }; 1113 var tio = TestIO{ .alloc = std.testing.allocator };
@@ -1114,7 +1133,7 @@ test "tick: sleep-until-flip times out after 2x blink_period_ns of real wait" {
1114 \\sleep-until-flip 1133 \\sleep-until-flip
1115 ); 1134 );
1116 defer s.deinit(); 1135 defer s.deinit();
1117 var state = ScenarioState.init(std.testing.allocator, &s, 0); 1136 var state = ScenarioState.init(std.testing.allocator, &s, 0, .{ .cell_w_px = 8, .cell_h_px = 16 });
1118 defer state.deinit(); 1137 defer state.deinit();
1119 1138
1120 var tio = TestIO{ .alloc = std.testing.allocator }; 1139 var tio = TestIO{ .alloc = std.testing.allocator };
@@ -1152,7 +1171,7 @@ test "tick: write_bytes callback failure surfaces as CallbackFailed" {
1152 \\bytes "x" 1171 \\bytes "x"
1153 ); 1172 );
1154 defer s.deinit(); 1173 defer s.deinit();
1155 var state = ScenarioState.init(std.testing.allocator, &s, 0); 1174 var state = ScenarioState.init(std.testing.allocator, &s, 0, .{ .cell_w_px = 8, .cell_h_px = 16 });
1156 defer state.deinit(); 1175 defer state.deinit();
1157 1176
1158 var ctx: u8 = 0; 1177 var ctx: u8 = 0;
@@ -1534,7 +1553,7 @@ test "tick+eval: assert-cell evaluates against last capture" {
1534 \\assert-cell 0 0 cursor-block-at 1553 \\assert-cell 0 0 cursor-block-at
1535 ); 1554 );
1536 defer s.deinit(); 1555 defer s.deinit();
1537 var state = ScenarioState.init(std.testing.allocator, &s, 0); 1556 var state = ScenarioState.init(std.testing.allocator, &s, 0, .{ .cell_w_px = 8, .cell_h_px = 16 });
1538 defer state.deinit(); 1557 defer state.deinit();
1539 1558
1540 var tio = TestIO{ .alloc = std.testing.allocator }; 1559 var tio = TestIO{ .alloc = std.testing.allocator };
@@ -1553,7 +1572,7 @@ test "tick+eval: assert-cell-at on missing label errors" {
1553 \\assert-cell-at nope 0 0 cell-empty 1572 \\assert-cell-at nope 0 0 cell-empty
1554 ); 1573 );
1555 defer s.deinit(); 1574 defer s.deinit();
1556 var state = ScenarioState.init(std.testing.allocator, &s, 0); 1575 var state = ScenarioState.init(std.testing.allocator, &s, 0, .{ .cell_w_px = 8, .cell_h_px = 16 });
1557 defer state.deinit(); 1576 defer state.deinit();
1558 1577
1559 var tio = TestIO{ .alloc = std.testing.allocator }; 1578 var tio = TestIO{ .alloc = std.testing.allocator };
@@ -1562,3 +1581,39 @@ test "tick+eval: assert-cell-at on missing label errors" {
1562 const r = state.tick(0, tio.io()); 1581 const r = state.tick(0, tio.io());
1563 try std.testing.expectError(error.PredicateOnMissingLabel, r); 1582 try std.testing.expectError(error.PredicateOnMissingLabel, r);
1564 } 1583 }
1584
1585 test "tick: duplicate capture label frees the prior image" {
1586 var s = try parseOk(
1587 \\size 80 24
1588 \\timeout 5000ms
1589 \\capture snap
1590 \\capture snap
1591 );
1592 defer s.deinit();
1593 var state = ScenarioState.init(std.testing.allocator, &s, 0, .{ .cell_w_px = 8, .cell_h_px = 16 });
1594 defer state.deinit();
1595
1596 var tio = TestIO{ .alloc = std.testing.allocator };
1597 defer tio.deinit();
1598
1599 _ = try state.tick(0, tio.io());
1600 try std.testing.expect(state.isDone());
1601 // Testing allocator will catch any leak here on deinit.
1602 }
1603
1604 test "tick: assert-cell with no prior capture errors distinctly" {
1605 var s = try parseOk(
1606 \\size 80 24
1607 \\timeout 5000ms
1608 \\assert-cell 0 0 cell-empty
1609 );
1610 defer s.deinit();
1611 var state = ScenarioState.init(std.testing.allocator, &s, 0, .{ .cell_w_px = 8, .cell_h_px = 16 });
1612 defer state.deinit();
1613
1614 var tio = TestIO{ .alloc = std.testing.allocator };
1615 defer tio.deinit();
1616
1617 const r = state.tick(0, tio.io());
1618 try std.testing.expectError(error.AssertCellWithoutCapture, r);
1619 }