Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 67 additions & 12 deletions src/ArenaPool.zig
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,33 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.

const std = @import("std");
const builtin = @import("builtin");

const Allocator = std.mem.Allocator;
const ArenaAllocator = std.heap.ArenaAllocator;

const ArenaPool = @This();

const IS_DEBUG = builtin.mode == .Debug;

allocator: Allocator,
retain_bytes: usize,
free_list_len: u16 = 0,
free_list: ?*Entry = null,
free_list_max: u16,
entry_pool: std.heap.MemoryPool(Entry),
mutex: std.Thread.Mutex = .{},
// Debug mode: track acquire/release counts per debug name to detect leaks and double-frees
_leak_track: if (IS_DEBUG) std.StringHashMapUnmanaged(isize) else void = if (IS_DEBUG) .empty else {},

const Entry = struct {
next: ?*Entry,
arena: ArenaAllocator,
debug: if (IS_DEBUG) []const u8 else void = if (IS_DEBUG) "" else {},
};

pub const DebugInfo = struct {
debug: []const u8 = "",
};

pub fn init(allocator: Allocator, free_list_max: u16, retain_bytes: usize) ArenaPool {
Expand All @@ -42,10 +52,26 @@ pub fn init(allocator: Allocator, free_list_max: u16, retain_bytes: usize) Arena
.free_list_max = free_list_max,
.retain_bytes = retain_bytes,
.entry_pool = .init(allocator),
._leak_track = if (IS_DEBUG) .empty else {},
};
}

pub fn deinit(self: *ArenaPool) void {
if (IS_DEBUG) {
var has_leaks = false;
var it = self._leak_track.iterator();
while (it.next()) |kv| {
if (kv.value_ptr.* != 0) {
std.debug.print("ArenaPool leak detected: '{s}' count={d}\n", .{ kv.key_ptr.*, kv.value_ptr.* });
has_leaks = true;
}
}
if (has_leaks) {
@panic("ArenaPool: leaked arenas detected");
}
self._leak_track.deinit(self.allocator);
}

var entry = self.free_list;
while (entry) |e| {
entry = e.next;
Expand All @@ -54,22 +80,38 @@ pub fn deinit(self: *ArenaPool) void {
self.entry_pool.deinit();
}

pub fn acquire(self: *ArenaPool) !Allocator {
pub fn acquire(self: *ArenaPool, dbg: DebugInfo) !Allocator {
self.mutex.lock();
defer self.mutex.unlock();

if (self.free_list) |entry| {
self.free_list = entry.next;
self.free_list_len -= 1;
if (IS_DEBUG) {
entry.debug = dbg.debug;
const gop = try self._leak_track.getOrPut(self.allocator, dbg.debug);
if (!gop.found_existing) {
gop.value_ptr.* = 0;
}
gop.value_ptr.* += 1;
}
return entry.arena.allocator();
}

const entry = try self.entry_pool.create();
entry.* = .{
.next = null,
.arena = ArenaAllocator.init(self.allocator),
.debug = if (IS_DEBUG) dbg.debug else {},
};

if (IS_DEBUG) {
const gop = try self._leak_track.getOrPut(self.allocator, dbg.debug);
if (!gop.found_existing) {
gop.value_ptr.* = 0;
}
gop.value_ptr.* += 1;
}
return entry.arena.allocator();
}

Expand All @@ -83,6 +125,19 @@ pub fn release(self: *ArenaPool, allocator: Allocator) void {
self.mutex.lock();
defer self.mutex.unlock();

if (IS_DEBUG) {
if (self._leak_track.getPtr(entry.debug)) |count| {
count.* -= 1;
if (count.* < 0) {
std.debug.print("ArenaPool double-free detected: '{s}'\n", .{entry.debug});
@panic("ArenaPool: double-free detected");
}
} else {
std.debug.print("ArenaPool release of untracked arena: '{s}'\n", .{entry.debug});
@panic("ArenaPool: release of untracked arena");
}
}

const free_list_len = self.free_list_len;
if (free_list_len == self.free_list_max) {
arena.deinit();
Expand All @@ -106,7 +161,7 @@ test "arena pool - basic acquire and use" {
var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16);
defer pool.deinit();

const alloc = try pool.acquire();
const alloc = try pool.acquire(.{ .debug = "test" });
const buf = try alloc.alloc(u8, 64);
@memset(buf, 0xAB);
try testing.expectEqual(@as(u8, 0xAB), buf[0]);
Expand All @@ -118,14 +173,14 @@ test "arena pool - reuse entry after release" {
var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16);
defer pool.deinit();

const alloc1 = try pool.acquire();
const alloc1 = try pool.acquire(.{ .debug = "test" });
try testing.expectEqual(@as(u16, 0), pool.free_list_len);

pool.release(alloc1);
try testing.expectEqual(@as(u16, 1), pool.free_list_len);

// The same entry should be returned from the free list.
const alloc2 = try pool.acquire();
const alloc2 = try pool.acquire(.{ .debug = "test" });
try testing.expectEqual(@as(u16, 0), pool.free_list_len);
try testing.expectEqual(alloc1.ptr, alloc2.ptr);

Expand All @@ -136,9 +191,9 @@ test "arena pool - multiple concurrent arenas" {
var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16);
defer pool.deinit();

const a1 = try pool.acquire();
const a2 = try pool.acquire();
const a3 = try pool.acquire();
const a1 = try pool.acquire(.{ .debug = "test1" });
const a2 = try pool.acquire(.{ .debug = "test2" });
const a3 = try pool.acquire(.{ .debug = "test3" });

// All three must be distinct arenas.
try testing.expect(a1.ptr != a2.ptr);
Expand All @@ -161,8 +216,8 @@ test "arena pool - free list respects max limit" {
var pool = ArenaPool.init(testing.allocator, 1, 1024 * 16);
defer pool.deinit();

const a1 = try pool.acquire();
const a2 = try pool.acquire();
const a1 = try pool.acquire(.{ .debug = "test1" });
const a2 = try pool.acquire(.{ .debug = "test2" });

pool.release(a1);
try testing.expectEqual(@as(u16, 1), pool.free_list_len);
Expand All @@ -176,7 +231,7 @@ test "arena pool - reset clears memory without releasing" {
var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16);
defer pool.deinit();

const alloc = try pool.acquire();
const alloc = try pool.acquire(.{ .debug = "test" });

const buf = try alloc.alloc(u8, 128);
@memset(buf, 0xFF);
Expand All @@ -200,8 +255,8 @@ test "arena pool - deinit with entries in free list" {
// detected by the test allocator).
var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16);

const a1 = try pool.acquire();
const a2 = try pool.acquire();
const a1 = try pool.acquire(.{ .debug = "test1" });
const a2 = try pool.acquire(.{ .debug = "test2" });
_ = try a1.alloc(u8, 256);
_ = try a2.alloc(u8, 512);
pool.release(a1);
Expand Down
6 changes: 5 additions & 1 deletion src/browser/Page.zig
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,11 @@ pub fn init(self: *Page, frame_id: u32, session: *Session, parent: ?*Page) !void
self._script_manager = ScriptManager.init(browser.allocator, browser.http_client, self);
errdefer self._script_manager.deinit();

self.js = try browser.env.createContext(self);
self.js = try browser.env.createContext(self, .{
.identity = &session.identity,
.identity_arena = session.page_arena,
.call_arena = self.call_arena,
});
errdefer self.js.deinit();

document._page = self;
Expand Down
106 changes: 47 additions & 59 deletions src/browser/Session.zig
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const log = @import("../log.zig");
const App = @import("../App.zig");

const js = @import("js/js.zig");
const v8 = js.v8;
const storage = @import("webapi/storage/storage.zig");
const Navigation = @import("webapi/navigation/Navigation.zig");
const History = @import("webapi/History.zig");
Expand Down Expand Up @@ -65,17 +66,14 @@ page_arena: Allocator,
// Origin map for same-origin context sharing. Scoped to the root page lifetime.
origins: std.StringHashMapUnmanaged(*js.Origin) = .empty,

// Identity tracking for the main world. All main world contexts share this,
// ensuring object identity works across same-origin frames.
identity: js.Identity = .{},

// Shared resources for all pages in this session.
// These live for the duration of the page tree (root + frames).
arena_pool: *ArenaPool,

// In Debug, we use this to see if anything fails to release an arena back to
// the pool.
_arena_pool_leak_track: if (IS_DEBUG) std.AutoHashMapUnmanaged(usize, struct {
owner: []const u8,
count: usize,
}) else void = if (IS_DEBUG) .empty else {},

page: ?Page,

queued_navigation: std.ArrayList(*Page),
Expand All @@ -84,17 +82,17 @@ queued_navigation: std.ArrayList(*Page),
// about:blank navigations (which may add to queued_navigation).
queued_queued_navigation: std.ArrayList(*Page),

page_id_gen: u32,
frame_id_gen: u32,
page_id_gen: u32 = 0,
frame_id_gen: u32 = 0,

pub fn init(self: *Session, browser: *Browser, notification: *Notification) !void {
const allocator = browser.app.allocator;
const arena_pool = browser.arena_pool;

const arena = try arena_pool.acquire();
const arena = try arena_pool.acquire(.{ .debug = "Session" });
errdefer arena_pool.release(arena);

const page_arena = try arena_pool.acquire();
const page_arena = try arena_pool.acquire(.{ .debug = "Session.page_arena" });
errdefer arena_pool.release(page_arena);

self.* = .{
Expand All @@ -104,8 +102,6 @@ pub fn init(self: *Session, browser: *Browser, notification: *Notification) !voi
.page_arena = page_arena,
.factory = Factory.init(page_arena),
.history = .{},
.page_id_gen = 0,
.frame_id_gen = 0,
// The prototype (EventTarget) for Navigation is created when a Page is created.
.navigation = .{ ._proto = undefined },
.storage_shed = .{},
Expand Down Expand Up @@ -171,32 +167,11 @@ pub const GetArenaOpts = struct {
};

pub fn getArena(self: *Session, opts: GetArenaOpts) !Allocator {
const allocator = try self.arena_pool.acquire();
if (comptime IS_DEBUG) {
// Use session's arena (not page_arena) since page_arena gets reset between pages
const gop = try self._arena_pool_leak_track.getOrPut(self.arena, @intFromPtr(allocator.ptr));
if (gop.found_existing and gop.value_ptr.count != 0) {
log.err(.bug, "ArenaPool Double Use", .{ .owner = gop.value_ptr.*.owner });
@panic("ArenaPool Double Use");
}
gop.value_ptr.* = .{ .owner = opts.debug, .count = 1 };
}
return allocator;
return self.arena_pool.acquire(.{ .debug = opts.debug });
}

pub fn releaseArena(self: *Session, allocator: Allocator) void {
if (comptime IS_DEBUG) {
const found = self._arena_pool_leak_track.getPtr(@intFromPtr(allocator.ptr)).?;
if (found.count != 1) {
log.err(.bug, "ArenaPool Double Free", .{ .owner = found.owner, .count = found.count });
if (comptime builtin.is_test) {
@panic("ArenaPool Double Free");
}
return;
}
found.count = 0;
}
return self.arena_pool.release(allocator);
self.arena_pool.release(allocator);
}

pub fn getOrCreateOrigin(self: *Session, key_: ?[]const u8) !*js.Origin {
Expand Down Expand Up @@ -237,18 +212,9 @@ pub fn releaseOrigin(self: *Session, origin: *js.Origin) void {
/// Reset page_arena and factory for a clean slate.
/// Called when root page is removed.
fn resetPageResources(self: *Session) void {
// Check for arena leaks before releasing
if (comptime IS_DEBUG) {
var it = self._arena_pool_leak_track.valueIterator();
while (it.next()) |value_ptr| {
if (value_ptr.count > 0) {
log.err(.bug, "ArenaPool Leak", .{ .owner = value_ptr.owner });
}
}
self._arena_pool_leak_track.clearRetainingCapacity();
}
self.identity.deinit();
self.identity = .{};

// All origins should have been released when contexts were destroyed
if (comptime IS_DEBUG) {
std.debug.assert(self.origins.count() == 0);
}
Expand All @@ -259,10 +225,9 @@ fn resetPageResources(self: *Session) void {
while (it.next()) |value| {
value.*.deinit(app);
}
self.origins.clearRetainingCapacity();
self.origins = .empty;
}

// Release old page_arena and acquire fresh one
self.frame_id_gen = 0;
self.arena_pool.reset(self.page_arena, 64 * 1024);
self.factory = Factory.init(self.page_arena);
Expand Down Expand Up @@ -632,16 +597,6 @@ fn processRootQueuedNavigation(self: *Session) !void {

defer self.arena_pool.release(qn.arena);

// HACK
// Mark as released in tracking BEFORE removePage clears the map.
// We can't call releaseArena() because that would also return the arena
// to the pool, making the memory invalid before we use qn.url/qn.opts.
if (comptime IS_DEBUG) {
if (self._arena_pool_leak_track.getPtr(@intFromPtr(qn.arena.ptr))) |found| {
found.count = 0;
}
}

self.removePage();

self.page = @as(Page, undefined);
Expand Down Expand Up @@ -672,3 +627,36 @@ pub fn nextPageId(self: *Session) u32 {
self.page_id_gen = id;
return id;
}

// A type that has a finalizer can have its finalizer called one of two ways.
// The first is from V8 via the WeakCallback we give to weakRef. But that isn't
// guaranteed to fire, so we track this in finalizer_callbacks and call them on
// page reset.
pub const FinalizerCallback = struct {
arena: Allocator,
session: *Session,
ptr: *anyopaque,
global: v8.Global,
identity: *js.Identity,
zig_finalizer: *const fn (ptr: *anyopaque, session: *Session) void,

pub fn deinit(self: *FinalizerCallback) void {
self.zig_finalizer(self.ptr, self.session);
self.session.releaseArena(self.arena);
}

/// Release this item from the identity tracking maps (called after finalizer runs from V8)
pub fn releaseIdentity(self: *FinalizerCallback) void {
const session = self.session;
const id = @intFromPtr(self.ptr);

if (self.identity.identity_map.fetchRemove(id)) |kv| {
var global = kv.value;
v8.v8__Global__Reset(&global);
}

_ = self.identity.finalizer_callbacks.remove(id);

session.releaseArena(self.arena);
}
};
Loading
Loading