From cd7b63cade4bd2f0552f2c19b70cb30ed78f2409 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Sun, 12 Jul 2026 19:31:03 +0800 Subject: [PATCH] mem: Finalize more (non-dom) types Adds finalizers to various types. The two most interesting are ImageData (which could have a large data field) and HTMLCollection which a page could create many. Various Crypto types are also finalized to make sure the key is freed. This is particularly important for freeing any keys created with EVP_PKEY_new. --- src/browser/tests/image_data.html | 13 ++++++ src/browser/webapi/CryptoKey.zig | 43 ++++++++++++++++++- src/browser/webapi/DOMNodeIterator.zig | 17 ++++++++ src/browser/webapi/DOMTreeWalker.zig | 17 ++++++++ src/browser/webapi/HTMLDocument.zig | 4 +- src/browser/webapi/ImageData.zig | 24 ++++++++++- src/browser/webapi/NodeFilter.zig | 6 +++ src/browser/webapi/SubtleCrypto.zig | 5 +-- .../webapi/collections/HTMLCollection.zig | 24 +++++++++++ .../HTMLFormControlsCollection.zig | 30 +++++++++++-- .../collections/HTMLOptionsCollection.zig | 18 ++++++++ src/browser/webapi/collections/NodeList.zig | 3 +- src/browser/webapi/crypto/AES.zig | 9 ++-- src/browser/webapi/crypto/EC.zig | 7 ++- src/browser/webapi/crypto/HMAC.zig | 15 +++---- src/browser/webapi/crypto/X25519.zig | 12 ++---- 16 files changed, 207 insertions(+), 40 deletions(-) diff --git a/src/browser/tests/image_data.html b/src/browser/tests/image_data.html index 3cf3282e8..94f3af1c2 100644 --- a/src/browser/tests/image_data.html +++ b/src/browser/tests/image_data.html @@ -70,6 +70,19 @@ } + + diff --git a/src/browser/webapi/CryptoKey.zig b/src/browser/webapi/CryptoKey.zig index f85c7ce20..12e7c119a 100644 --- a/src/browser/webapi/CryptoKey.zig +++ b/src/browser/webapi/CryptoKey.zig @@ -16,16 +16,22 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . +const std = @import("std"); +const lp = @import("lightpanda"); const crypto = @import("../../sys/libcrypto.zig"); const js = @import("../js/js.zig"); +const Page = @import("../Page.zig"); const Execution = js.Execution; +const Allocator = std.mem.Allocator; /// Represents a cryptographic key obtained from one of the SubtleCrypto methods /// generateKey(), deriveKey(), importKey(), or unwrapKey(). const CryptoKey = @This(); +_rc: lp.RC(u8) = .{}, +_arena: Allocator = undefined, /// Algorithm being used. _type: Type, /// Whether this is a secret (symmetric), public, or private key. Surfaced as @@ -37,8 +43,8 @@ _extractable: bool, _usages: u8, /// Raw bytes of key. _key: []const u8, -/// Metadata needed to reconstruct the JS `.algorithm` dictionary. The strings -/// are expected to outlive the key (arena-allocated alongside it). +/// Metadata needed to reconstruct the JS `.algorithm` dictionary. `hash` is +/// duped into the key's arena by init; `name`/`named_curve` are static. _algorithm: Algorithm, /// Different algorithms may use different data structures; /// this union can be used for such situations. Active field is understood @@ -94,6 +100,39 @@ pub const Usages = struct { // zig fmt: on }; +/// Copies `key` into an owned pooled arena, duping the caller-provided +/// slices (`_key`, `_algorithm.hash`). `_algorithm.name` and `_named_curve` +/// are expected to be static strings. Takes ownership of `_vary.pkey`. +pub fn init(exec: *const Execution, key: CryptoKey) !*CryptoKey { + const arena = try exec.getArena(.tiny, "CryptoKey"); + errdefer exec.releaseArena(arena); + + const self = try arena.create(CryptoKey); + self.* = key; + self._arena = arena; + self._key = try arena.dupe(u8, key._key); + if (key._algorithm.hash) |hash| { + self._algorithm.hash = try arena.dupe(u8, hash); + } + return self; +} + +pub fn deinit(self: *CryptoKey, page: *Page) void { + switch (self._vary) { + .pkey => |pkey| crypto.EVP_PKEY_free(pkey), + .none, .digest => {}, + } + page.releaseArena(self._arena); +} + +pub fn releaseRef(self: *CryptoKey, page: *Page) void { + self._rc.release(self, page); +} + +pub fn acquireRef(self: *CryptoKey) void { + self._rc.acquire(); +} + pub fn canEncrypt(self: *const CryptoKey) bool { return self._usages & Usages.encrypt != 0; } diff --git a/src/browser/webapi/DOMNodeIterator.zig b/src/browser/webapi/DOMNodeIterator.zig index 6756e4d75..9951155ce 100644 --- a/src/browser/webapi/DOMNodeIterator.zig +++ b/src/browser/webapi/DOMNodeIterator.zig @@ -16,7 +16,10 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . +const lp = @import("lightpanda"); + const js = @import("../js/js.zig"); +const Page = @import("../Page.zig"); const Frame = @import("../Frame.zig"); const Node = @import("Node.zig"); @@ -25,6 +28,7 @@ pub const FilterOpts = NodeFilter.FilterOpts; const DOMNodeIterator = @This(); +_rc: lp.RC(u8) = .{}, _root: *Node, _what_to_show: u32, _filter: NodeFilter, @@ -43,6 +47,19 @@ pub fn init(root: *Node, what_to_show: u32, filter: ?FilterOpts, frame: *Frame) }); } +pub fn deinit(self: *DOMNodeIterator, page: *Page) void { + self._filter.deinit(); + page.factory.destroy(self); +} + +pub fn releaseRef(self: *DOMNodeIterator, page: *Page) void { + self._rc.release(self, page); +} + +pub fn acquireRef(self: *DOMNodeIterator) void { + self._rc.acquire(); +} + pub fn getRoot(self: *const DOMNodeIterator) *Node { return self._root; } diff --git a/src/browser/webapi/DOMTreeWalker.zig b/src/browser/webapi/DOMTreeWalker.zig index 29779fa68..6c5d616a4 100644 --- a/src/browser/webapi/DOMTreeWalker.zig +++ b/src/browser/webapi/DOMTreeWalker.zig @@ -16,7 +16,10 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . +const lp = @import("lightpanda"); + const js = @import("../js/js.zig"); +const Page = @import("../Page.zig"); const Frame = @import("../Frame.zig"); const Node = @import("Node.zig"); @@ -25,6 +28,7 @@ pub const FilterOpts = NodeFilter.FilterOpts; const DOMTreeWalker = @This(); +_rc: lp.RC(u8) = .{}, _root: *Node, _what_to_show: u32, _filter: NodeFilter, @@ -40,6 +44,19 @@ pub fn init(root: *Node, what_to_show: u32, filter: ?FilterOpts, frame: *Frame) }); } +pub fn deinit(self: *DOMTreeWalker, page: *Page) void { + self._filter.deinit(); + page.factory.destroy(self); +} + +pub fn releaseRef(self: *DOMTreeWalker, page: *Page) void { + self._rc.release(self, page); +} + +pub fn acquireRef(self: *DOMTreeWalker) void { + self._rc.acquire(); +} + pub fn getRoot(self: *const DOMTreeWalker) *Node { return self._root; } diff --git a/src/browser/webapi/HTMLDocument.zig b/src/browser/webapi/HTMLDocument.zig index d8e5a757f..f3223fee6 100644 --- a/src/browser/webapi/HTMLDocument.zig +++ b/src/browser/webapi/HTMLDocument.zig @@ -188,8 +188,8 @@ pub fn getEmbeds(self: *HTMLDocument, frame: *Frame) !collections.NodeLive(.tag) return collections.NodeLive(.tag).init(self.asNode(), .embed, frame); } -pub fn getApplets(_: *const HTMLDocument) collections.HTMLCollection { - return .{ ._data = .empty }; +pub fn getApplets(_: *const HTMLDocument, frame: *Frame) !*collections.HTMLCollection { + return frame._factory.create(collections.HTMLCollection{ ._data = .empty }); } pub fn getCurrentScript(self: *const HTMLDocument) ?*Element.Html.Script { diff --git a/src/browser/webapi/ImageData.zig b/src/browser/webapi/ImageData.zig index 37c387c18..cca2b5212 100644 --- a/src/browser/webapi/ImageData.zig +++ b/src/browser/webapi/ImageData.zig @@ -27,6 +27,8 @@ const Execution = js.Execution; /// https://developer.mozilla.org/en-US/docs/Web/API/ImageData/ImageData const ImageData = @This(); + +_rc: lp.RC(u8) = .{}, _width: u32, _height: u32, _data: js.ArrayBufferRef(.uint8_clamped).Global, @@ -74,9 +76,14 @@ pub fn init( } var size, var overflown = @mulWithOverflow(width, height); - if (overflown == 1) return error.IndexSizeError; + if (overflown == 1) { + return error.IndexSizeError; + } + size, overflown = @mulWithOverflow(size, 4); - if (overflown == 1) return error.IndexSizeError; + if (overflown == 1) { + return error.IndexSizeError; + } return exec._factory.create(ImageData{ ._width = width, @@ -85,6 +92,19 @@ pub fn init( }); } +pub fn deinit(self: *ImageData, page: *Page) void { + self._data.release(); + page.factory.destroy(self); +} + +pub fn releaseRef(self: *ImageData, page: *Page) void { + self._rc.release(self, page); +} + +pub fn acquireRef(self: *ImageData) void { + self._rc.acquire(); +} + pub fn structuredSerialize(self: *const ImageData, writer: *js.StructuredWriter) !void { writer.writeUint32(self._width); writer.writeUint32(self._height); diff --git a/src/browser/webapi/NodeFilter.zig b/src/browser/webapi/NodeFilter.zig index a2519e718..b0abe9e23 100644 --- a/src/browser/webapi/NodeFilter.zig +++ b/src/browser/webapi/NodeFilter.zig @@ -44,6 +44,12 @@ pub fn init(opts_: ?FilterOpts) !NodeFilter { }; } +pub fn deinit(self: *const NodeFilter) void { + if (self._func) |func| { + func.release(); + } +} + // Constants pub const FILTER_ACCEPT: i32 = 1; pub const FILTER_REJECT: i32 = 2; diff --git a/src/browser/webapi/SubtleCrypto.zig b/src/browser/webapi/SubtleCrypto.zig index cc2d0465c..8743aed04 100644 --- a/src/browser/webapi/SubtleCrypto.zig +++ b/src/browser/webapi/SubtleCrypto.zig @@ -205,13 +205,12 @@ pub fn importKey( const mask = common.usageMask(&.{ "deriveKey", "deriveBits" }, key_usages) catch |err| { return local.rejectPromise(.{ .dom_exception = .{ .err = err } }); }; - const key = try exec.arena.dupe(u8, raw); - const crypto_key = try exec._factory.create(CryptoKey{ + const crypto_key = try CryptoKey.init(exec, .{ ._type = .derive, ._kind = .secret, ._extractable = extractable, ._usages = mask, - ._key = key, + ._key = raw, ._algorithm = .{ .name = derive_name }, }); return local.resolvePromise(crypto_key); diff --git a/src/browser/webapi/collections/HTMLCollection.zig b/src/browser/webapi/collections/HTMLCollection.zig index e67395945..83f0224c5 100644 --- a/src/browser/webapi/collections/HTMLCollection.zig +++ b/src/browser/webapi/collections/HTMLCollection.zig @@ -17,7 +17,10 @@ // along with this program. If not, see . const std = @import("std"); +const lp = @import("lightpanda"); + const js = @import("../../js/js.zig"); +const Page = @import("../../Page.zig"); const Frame = @import("../../Frame.zig"); const Element = @import("../Element.zig"); const TreeWalker = @import("../TreeWalker.zig"); @@ -55,6 +58,19 @@ _data: union(Mode) { form: NodeLive(.form), empty: void, }, +_rc: lp.RC(u8) = .{}, + +pub fn deinit(self: *HTMLCollection, page: *Page) void { + page.factory.destroy(self); +} + +pub fn releaseRef(self: *HTMLCollection, page: *Page) void { + self._rc.release(self, page); +} + +pub fn acquireRef(self: *HTMLCollection) void { + self._rc.acquire(); +} pub fn length(self: *HTMLCollection, frame: *const Frame) u32 { return switch (self._data) { @@ -119,6 +135,14 @@ pub const Iterator = GenericIterator(struct { empty: void, }, + pub fn acquireRef(self: *@This()) void { + self.list.acquireRef(); + } + + pub fn releaseRef(self: *@This(), page: *Page) void { + self.list.releaseRef(page); + } + pub fn next(self: *@This(), _: *const Execution) ?*Element { return switch (self.list._data) { .tag => |*impl| impl.nextTw(&self.tw.tag), diff --git a/src/browser/webapi/collections/HTMLFormControlsCollection.zig b/src/browser/webapi/collections/HTMLFormControlsCollection.zig index 2147e8131..ce03f9581 100644 --- a/src/browser/webapi/collections/HTMLFormControlsCollection.zig +++ b/src/browser/webapi/collections/HTMLFormControlsCollection.zig @@ -18,6 +18,7 @@ const std = @import("std"); const js = @import("../../js/js.zig"); +const Page = @import("../../Page.zig"); const Frame = @import("../../Frame.zig"); const Element = @import("../Element.zig"); @@ -31,10 +32,22 @@ const HTMLFormControlsCollection = @This(); _proto: *HTMLCollection, -pub const NamedItemResult = union(enum) { - element: *Element, - radio_node_list: *RadioNodeList, -}; +// The refcount lives on the proto, but anchoring the finalizer here lets +// deinit reclaim this struct's slot along with the proto's. +pub fn deinit(self: *HTMLFormControlsCollection, page: *Page) void { + self._proto.deinit(page); + // Not destroy(): the proto is a separate slab allocation, not a + // contiguous factory chain. + page.factory.destroyStandalone(self); +} + +pub fn acquireRef(self: *HTMLFormControlsCollection) void { + self._proto.acquireRef(); +} + +pub fn releaseRef(self: *HTMLFormControlsCollection, page: *Page) void { + self._proto._rc.release(self, page); +} pub fn length(self: *HTMLFormControlsCollection, frame: *Frame) u32 { return self._proto.length(frame); @@ -44,6 +57,11 @@ pub fn getAtIndex(self: *HTMLFormControlsCollection, index: usize, frame: *Frame return self._proto.getAtIndex(index, frame); } +pub const NamedItemResult = union(enum) { + element: *Element, + radio_node_list: *RadioNodeList, +}; + pub fn namedItem(self: *HTMLFormControlsCollection, name: []const u8, frame: *Frame) !?NamedItemResult { if (name.len == 0) { return null; @@ -87,6 +105,10 @@ pub fn namedItem(self: *HTMLFormControlsCollection, name: []const u8, frame: *Fr radio_node_list._proto = try frame._factory.create(NodeList{ ._data = .{ .radio_node_list = radio_node_list } }); + // The RadioNodeList outlives this call; its NodeList releases + // the ref in deinit. + self.acquireRef(); + return .{ .radio_node_list = radio_node_list }; } } diff --git a/src/browser/webapi/collections/HTMLOptionsCollection.zig b/src/browser/webapi/collections/HTMLOptionsCollection.zig index 83a837b06..0af002107 100644 --- a/src/browser/webapi/collections/HTMLOptionsCollection.zig +++ b/src/browser/webapi/collections/HTMLOptionsCollection.zig @@ -17,6 +17,7 @@ // along with this program. If not, see . const js = @import("../../js/js.zig"); +const Page = @import("../../Page.zig"); const Frame = @import("../../Frame.zig"); const Node = @import("../Node.zig"); const Element = @import("../Element.zig"); @@ -27,6 +28,23 @@ const HTMLOptionsCollection = @This(); _proto: *HTMLCollection, _select: *@import("../element/html/Select.zig"), +// The refcount lives on the proto, but anchoring the finalizer here lets +// deinit reclaim this struct's slot along with the proto's. +pub fn deinit(self: *HTMLOptionsCollection, page: *Page) void { + self._proto.deinit(page); + // Not destroy(): the proto is a separate slab allocation, not a + // contiguous factory chain. + page.factory.destroyStandalone(self); +} + +pub fn acquireRef(self: *HTMLOptionsCollection) void { + self._proto.acquireRef(); +} + +pub fn releaseRef(self: *HTMLOptionsCollection, page: *Page) void { + self._proto._rc.release(self, page); +} + // Forward length to HTMLCollection pub fn length(self: *HTMLOptionsCollection, frame: *Frame) u32 { return self._proto.length(frame); diff --git a/src/browser/webapi/collections/NodeList.zig b/src/browser/webapi/collections/NodeList.zig index 6a768a555..eb9cb6901 100644 --- a/src/browser/webapi/collections/NodeList.zig +++ b/src/browser/webapi/collections/NodeList.zig @@ -45,7 +45,8 @@ pub fn deinit(self: *NodeList, page: *Page) void { switch (self._data) { .child_nodes => |cn| cn.deinit(page), .selector_list => |list| list.deinit(page), - else => {}, + .radio_node_list => |rnl| rnl._form_collection.releaseRef(page), + .name => {}, } } diff --git a/src/browser/webapi/crypto/AES.zig b/src/browser/webapi/crypto/AES.zig index 3114671ef..63005f0c0 100644 --- a/src/browser/webapi/crypto/AES.zig +++ b/src/browser/webapi/crypto/AES.zig @@ -93,12 +93,12 @@ pub fn generate( const allowed = allowedUsages(params.name).?; const mask = common.usageMask(allowed, key_usages) catch unreachable; - const key = try exec.arena.alloc(u8, params.length / 8); + const key = try exec.local_arena.alloc(u8, params.length / 8); const res = crypto.RAND_bytes(key.ptr, key.len); lp.assert(res == 1, "AES.generate", .{ .res = res }); - const crypto_key = try exec._factory.create(CryptoKey{ + const crypto_key = try CryptoKey.init(exec, .{ ._type = .aes, ._kind = .secret, ._extractable = extractable, @@ -133,13 +133,12 @@ pub fn import( return local.rejectPromise(.{ .dom_exception = .{ .err = error.DataError } }); } - const key = try exec.arena.dupe(u8, raw); - const crypto_key = try exec._factory.create(CryptoKey{ + const crypto_key = try CryptoKey.init(exec, .{ ._type = .aes, ._kind = .secret, ._extractable = extractable, ._usages = mask, - ._key = key, + ._key = raw, ._algorithm = .{ .name = canonical }, }); diff --git a/src/browser/webapi/crypto/EC.zig b/src/browser/webapi/crypto/EC.zig index ab1f753ba..6e249e222 100644 --- a/src/browser/webapi/crypto/EC.zig +++ b/src/browser/webapi/crypto/EC.zig @@ -127,7 +127,7 @@ pub fn generate( errdefer crypto.EVP_PKEY_free(public_pkey); if (crypto.EVP_PKEY_set1_EC_KEY(public_pkey, pub_ec) != 1) return error.OutOfMemory; - const private = try exec._factory.create(CryptoKey{ + const private = try CryptoKey.init(exec, .{ ._type = .ec, ._kind = .private, ._extractable = extractable, @@ -136,9 +136,8 @@ pub fn generate( ._algorithm = .{ .name = name, .named_curve = curve }, ._vary = .{ .pkey = private_pkey }, }); - errdefer exec._factory.destroy(private); - const public = try exec._factory.create(CryptoKey{ + const public = try CryptoKey.init(exec, .{ ._type = .ec, ._kind = .public, // Public keys are always extractable. @@ -192,7 +191,7 @@ pub fn import( return local.rejectPromise(.{ .dom_exception = .{ .err = error.DataError } }); } - const crypto_key = try exec._factory.create(CryptoKey{ + const crypto_key = try CryptoKey.init(exec, .{ ._type = .ec, ._kind = if (is_private) .private else .public, ._extractable = extractable, diff --git a/src/browser/webapi/crypto/HMAC.zig b/src/browser/webapi/crypto/HMAC.zig index 625ff7cbf..475b6553b 100644 --- a/src/browser/webapi/crypto/HMAC.zig +++ b/src/browser/webapi/crypto/HMAC.zig @@ -81,18 +81,18 @@ pub fn init( } // Should we reject this in promise too? - const key = try exec.arena.alloc(u8, block_size); + const key = try exec.local_arena.alloc(u8, block_size); // HMAC is simply CSPRNG. const res = crypto.RAND_bytes(key.ptr, key.len); lp.assert(res == 1, "HMAC.init", .{ .res = res }); - const crypto_key = try exec._factory.create(CryptoKey{ + const crypto_key = try CryptoKey.init(exec, .{ ._type = .hmac, ._extractable = extractable, ._usages = mask, ._key = key, - ._algorithm = .{ .name = "HMAC", .hash = try exec.arena.dupe(u8, hash_name) }, + ._algorithm = .{ .name = "HMAC", .hash = hash_name }, ._vary = .{ .digest = digest }, }); @@ -122,16 +122,13 @@ pub fn import( return local.rejectPromise(.{ .dom_exception = .{ .err = error.DataError } }); } - const key = try exec.arena.dupe(u8, raw); - errdefer exec.arena.free(key); - - const crypto_key = try exec._factory.create(CryptoKey{ + const crypto_key = try CryptoKey.init(exec, .{ ._type = .hmac, ._kind = .secret, ._extractable = extractable, ._usages = mask, - ._key = key, - ._algorithm = .{ .name = "HMAC", .hash = try exec.arena.dupe(u8, hash_name) }, + ._key = raw, + ._algorithm = .{ .name = "HMAC", .hash = hash_name }, ._vary = .{ .digest = digest }, }); diff --git a/src/browser/webapi/crypto/X25519.zig b/src/browser/webapi/crypto/X25519.zig index 631b6687c..adaa09bf7 100644 --- a/src/browser/webapi/crypto/X25519.zig +++ b/src/browser/webapi/crypto/X25519.zig @@ -60,11 +60,8 @@ pub fn init( }); } - const public_value = try exec.arena.alloc(u8, crypto.X25519_PUBLIC_VALUE_LEN); - errdefer exec.arena.free(public_value); - - const private_key = try exec.arena.alloc(u8, crypto.X25519_PRIVATE_KEY_LEN); - errdefer exec.arena.free(private_key); + const public_value = try exec.local_arena.alloc(u8, crypto.X25519_PUBLIC_VALUE_LEN); + const private_key = try exec.local_arena.alloc(u8, crypto.X25519_PRIVATE_KEY_LEN); // There's no info about whether this can fail; so I assume it cannot. crypto.X25519_keypair(@ptrCast(public_value), @ptrCast(private_key)); @@ -91,7 +88,7 @@ pub fn init( private_key.len, ) orelse return error.OutOfMemory; - const private = try exec._factory.create(CryptoKey{ + const private = try CryptoKey.init(exec, .{ ._type = .x25519, ._kind = .private, ._extractable = extractable, @@ -100,9 +97,8 @@ pub fn init( ._algorithm = .{ .name = "X25519" }, ._vary = .{ .pkey = private_pkey }, }); - errdefer exec._factory.destroy(private); - const public = try exec._factory.create(CryptoKey{ + const public = try CryptoKey.init(exec, .{ ._type = .x25519, ._kind = .public, // Public keys are always extractable.