Merged
Conversation
js.Origin was added to allow frames on the same origin to share our zig<->js maps / identity. It assumes that scripts on different origins will never be allowed (by v8) to access the same zig instances. If two different origins DID access the same zig instance, we'd have a few different problems. First, while the mapping would exist in Origin1's identity_map, when the zig instance was returned to a script in Origin2, it would not be found in Origin2's identity_map, and thus create a new v8::Object. Thus we'd end up with 2 v8::Objects for the same Zig instance. This is potentially not the end of the world, but not great either as any zig-native data _would_ be shared (it's the same instance after all), but js-native data wouldn't. The real problem this introduces though is with Finalizers. A weak reference that falls out of scope in Origin1 will get cleaned up, even though it's still referenced from Origin2. Now, under normal circumstances, this isn't an issue; v8 _does_ ensure that cross-origin access isn't allowed (because we set a SecurityToken on the v8::Context). But it seems like the v8::Inspector isn't bound by these restrictions and can happily access and share objects across origin. The simplest solution I can come up with is to move the mapping from the Origin to the Session. This does mean that objects might live longer than they have to. When all references to an origin go out of scope, we can do some cleanup. Not so when the Session owns this data. But really, how often are iframes on different origins being created and deleted within the lifetime of a page? When Origins were first introduces, the Session got burdened with having to manage multiple lifecycles: 1 - The page-surviving data (e.g. history) 2 - The root page lifecycle (e.g. page_arena, queuedNavigation) 3 - The origin lookup This commit doesn't change that, but it makes the session responsible for _a lot_ more of the root page lifecycle (#2 above). I lied. js.Origin still exists, but it's a shell of its former self. It only exists to store the SecurityToken name that is re-used for every context with the same origin. The v8 namespace leaks into Session. MutationObserver and IntersectionObserver are now back to using weak/strong refs which was one of the failing cases before this change.
8d57782 to
d9c5f56
Compare
History: We started with 1 context and thus only had 1 identity map. Frames were added, and we tried to stick with 1 identity map per context. That didn't work - it breaks cross-frame scripting. We introduced "Origin" so that all frames on the same origin share the same objects. That almost worked, by the v8::Inspector isn't bound by a Context's SecurityToken. So we tried 1 global identity map. But that doesn't work. CDP IsolateWorlds do, in fact, need some isolation. They need new v8::Objects created in their context, even if the object already exists in the main context. In the end, you end up with something like this: A page (and all its frames) needs 1 view of the data. And each IsolateWorld needs it own view. This commit introduces a js.Identity which is referenced by the context. The Session has a js.Identity (used by all pages), and each IsolateWorld has its own js.Identity. As a bonus, the arena pool memory-leak detection has been moved out of the session and into the ArenaPool. This means _all_ arena pool access is audited (in debug mode). This seems superfluous, but it's actually necessary since IsolateWorlds (which now own their own identity) can outlive the Page so there's no clear place to "check" for leaks - except on ArenaPool deinit.
The Context's call_arena should be based on the source, e.g. the IsolateWorld or the Page, not always the page. There's no rule that says all Contexts have to be a subset of the Page, and thus some might live longer and by doing so outlive the page_arena. Also, on context cleanup, isolate worlds now cleanup their identity.
krichprollsch
approved these changes
Mar 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
js.Origin was added to allow frames on the same origin to share our zig<->js maps / identity. It assumes that scripts on different origins will never be allowed (by v8) to access the same zig instances.
If two different origins DID access the same zig instance, we'd have a few different problems. First, while the mapping would exist in Origin1's identity_map, when the zig instance was returned to a script in Origin2, it would not be found in Origin2's identity_map, and thus create a new v8::Object. Thus we'd end up with 2 v8::Objects for the same Zig instance. This is potentially not the end of the world, but not great either as any zig-native data would be shared (it's the same instance after all), but js-native data wouldn't.
The real problem this introduces though is with Finalizers. A weak reference that falls out of scope in Origin1 will get cleaned up, even though it's still referenced from Origin2.
Now, under normal circumstances, this isn't an issue; v8 does ensure that cross-origin access isn't allowed (because we set a SecurityToken on the v8::Context). But it seems like the v8::Inspector isn't bound by these restrictions and can happily access and share objects across origin.
The simplest solution I can come up with is to move the mapping from the Origin to the Session. This does mean that objects might live longer than they have to. When all references to an origin go out of scope, we can do some cleanup. Not so when the Session owns this data. But really, how often are iframes on different origins being created and deleted within the lifetime of a page?
When Origins were first introduces, the Session got burdened with having to manage multiple lifecycles:
1 - The page-surviving data (e.g. history)
2 - The root page lifecycle (e.g. page_arena, queuedNavigation) 3 - The origin lookup
This commit doesn't change that, but it makes the session responsible for a lot more of the root page lifecycle (#2 above).
I lied. js.Origin still exists, but it's a shell of its former self. It only exists to store the SecurityToken name that is re-used for every context with the same origin.
The v8 namespace leaks into Session.
MutationObserver and IntersectionObserver are now back to using weak/strong refs which was one of the failing cases before this change.