Skip to content

Conversation

@karlseguin
Copy link
Collaborator

This uncovered a bug in the js_object_map which, in retrospect, is a bit obvious. There's nothing stopping v8 from re-using objects/addresses, so the @intFromPtr(obj.handle) cannot safely be used as a key in an identity map (well, I say it's obvious, but I'm still not sure about re-using the object/address after we've created a Persistent wrapper around it...). Instead of creating a lookup, we just always created the persisted version, possibly creating duplicates (which I don't see as an issue really).

There's more to message passing than the MessageChannel and MessagePort, e.g. window.postMessage), but little steps.

@karlseguin karlseguin merged commit 5bdacba into main Jul 21, 2025
10 checks passed
@karlseguin karlseguin deleted the MessageChannel branch July 21, 2025 23:13
@github-actions github-actions bot locked and limited conversation to collaborators Jul 21, 2025
@krichprollsch
Copy link
Member

Instead of creating a lookup, we just always created the persisted version, possibly creating duplicates (which I don't see as an issue really).

I'm not sure about the impact of this change. duplicating the persistent objects is not an issue you think?

@karlseguin
Copy link
Collaborator Author

It took a while to figure out, because the behavior was surprising. But I think the older v8 docs hint at what I was seeing:

A persistent handle contains a reference to a storage cell within the v8 engine which holds an object value and which is updated by the garbage collector whenever the object is moved

So using the v8.Object's handle as the key/identity was never correct/safe. Given a v8.Object that represents some JS variable (e.g. let x= document.createElement('div')). The address of that object was never guaranteed not to move [between function calls].

As for having duplicates..first, I don't see a solution, but the [older] documentation implies this is fine:

Since persistent handles are passed by value you may have many persistent handle objects that point to the same storage cell.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants