diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 61b85b7d8..f26748c35 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -642,8 +642,7 @@ fn verifyIntegrity(self: *const PageList) IntegrityError!void { } } -/// Deinit the pagelist. If you own the memory pool (used clonePool) then -/// this will reset the pool and retain capacity. +/// Deinit the pagelist, freeing all page memory and the memory pool. pub fn deinit(self: *PageList) void { // Verify integrity before cleanup self.assertIntegrity(); @@ -3499,6 +3498,14 @@ inline fn createPageExt( // In runtime safety modes, allocators fill with 0xAA. On freestanding // (WASM), the WasmAllocator reuses freed slots without zeroing. + // + // Otherwise, we rely on pool item buffers being zeroed: fresh items + // come from the OS page allocator (zeroed pages) and destroyNodeExt + // zeroes buffers before returning them to the pool. The one + // exception is the first pointer-size bytes, which hold the pool's + // free list node while a buffer is in the free list; initBuf below + // always overwrites those since the page rows start at offset 0 + // (comptime-asserted in Page). if (comptime std.debug.runtime_safety or builtin.os.tag == .freestanding) @memset(page_buf, 0); diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index b9d421982..5ecfe7933 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -10181,9 +10181,9 @@ test "Screen: cursorDown to page with insufficient capacity" { // cursor movement functions). The bug pattern: // // 1. cursorDown creates a by-value copy of the pin via page_pin.down(n) - // 2. cursorChangePin is called, which may trigger adjustCapacity + // 2. cursorChangePin is called, which may trigger increaseCapacity // if the target page's style map is full - // 3. adjustCapacity frees the old page and creates a new one + // 3. increaseCapacity frees the old page and creates a new one // 4. The local pin copy still points to the freed page // 5. rowAndCell() on the stale pin accesses freed memory @@ -10206,7 +10206,7 @@ test "Screen: cursorDown to page with insufficient capacity" { try testing.expect(start_page != new_page); // Fill new_page's style map to capacity. When we move INTO this page - // with a style set, adjustCapacity will be triggered. + // with a style set, increaseCapacity will be triggered. { new_page.pauseIntegrityChecks(true); defer new_page.pauseIntegrityChecks(false); @@ -10235,7 +10235,7 @@ test "Screen: cursorDown to page with insufficient capacity" { try testing.expect(&next_pin.node.data == new_page); // This cursorDown triggers the bug: the local page_pin copy - // becomes stale after adjustCapacity, causing rowAndCell() + // becomes stale after increaseCapacity, causing rowAndCell() // to access freed memory. s.cursorDown(1); diff --git a/src/terminal/page.zig b/src/terminal/page.zig index 803e67c8f..c2759d2a2 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -145,6 +145,15 @@ pub const Page = struct { @alignOf(Cell), StyleSet.base_align.toByteUnits(), ) == 0); + + // The PageList memory pool requires that initBuf overwrites at + // least the first pointer-size bytes of the backing buffer: + // std.heap.MemoryPool stores its free list node there when a + // page buffer is returned to it, and pool reuse skips zeroing + // in release builds. This holds because the rows array is at + // offset 0 (see layout), a page always has at least one row, + // and initBuf fully rewrites every row. + assert(@sizeOf(Row) >= @sizeOf(usize)); } /// The backing memory for the page. A page is always made up of a @@ -238,6 +247,13 @@ pub const Page = struct { /// It is up to the caller to not call deinit on these pages. pub inline fn initBuf(buf: OffsetBuf, l: Layout) Page { const cap = l.capacity; + + // A page must always have at least one row. Aside from being + // useless otherwise, the row initialization below must always + // overwrite the start of the buffer for pool reuse. See the + // comptime assert at the top of Page. + assert(cap.rows > 0); + const rows = buf.member(Row, l.rows_start); const cells = buf.member(Cell, l.cells_start); @@ -1703,6 +1719,11 @@ pub const Page = struct { /// and rows size. pub inline fn layout(cap: Capacity) Layout { const rows_count: usize = @intCast(cap.rows); + + // The rows array must stay at offset 0: the PageList memory + // pool relies on initBuf overwriting the first bytes of a + // reused page buffer, which hold the pool's free list node. + // See the comptime assert at the top of Page. const rows_start = 0; const rows_end: usize = rows_start + (rows_count * @sizeOf(Row));