read_blob returns a reflector to an unrooted DOM object

[jdm]

This code creates a Blob, which is rooted in the current stack frame, then returns its reflector's JSObject pointer. This means that the blob object is no longer rooted, and depending on what the JS engine does with the return value of read_callback, the pointer could be pointing at a DOM object that gets GCed by the time it's used. We should look carefully at what Firefox does here.

[gterzian]

Is the rooting handled at the point of use? For example the below, where data is StructuredCloneData:

servo/components/script/dom/dedicatedworkerglobalscope.rs

Line 316 in 9149524

data.read(scope.upcast(), message.handle_mut());

[gterzian]

Or do you mean literally that the GC would happen 'right in the middle' such a call? Perhaps this is protected by the fact that the "reading" happens inside of this:

servo/components/script/dom/bindings/structuredclone.rs

Line 248 in 8eb2a53

assert!(JS_ReadStructuredClone(cx,

I also remember pretty much copy pasting the whole structure from gecko when I worked on this :) (although perhaps not creating the blob just to return the pointer)

[gterzian]

Lastly, I remember reading through the actual implementation of JS_ReadStructuredClone(for some reason it seemed like the SpiderMonkey code was right inside the Gecko codebase), and the way it works is that the JS engine will try to internally handle the cloning, and only if it doesn't recognize the type of the clone will it invoke the read_callback. It seemed to all happen 'in sync' too, so there is no ayncness involved. I would assume that the JS_ReadStructuredClone itself would function as some sort of lock and ensure there is no GC in the meantime...

[jdm]

Yes, there is no risk of GC happening randomly or in parallel. It can happen as a result of any new JS allocation or certain API calls.

[gterzian]

In Gecko, they actually seem to keep track of cloned Blobs with this nsTArray<RefPtr> mBlobImplArray, stored inside a StructuredCloneHolder.

So what they do is also 'creating a blob and then returning a pointer to it's raw JsObject', however they always keep the corresponding BlobImpl in this Array(This BlobImpl I guess corresponds to a DomRoot<Blob> in our case?).

The StructuredCloneHolder is available from the callbacks, because they pass it around as the closure argument when calling JS_WriteStructuredClone/JS_ReadStructuredClone.

In the callbacks, they write/read the index of a given Blob in the array using JS_WriteBytes/JS_ReadBytes(see here), so that they can access it later.

So perhaps we could store the DomRoot<Blob> inside a StructuredCloneHolder as well, and wrap it around the current StructuredCloneData, at least for cases were the callbacks will be invoked.

Actually we might want to remove the callbacks from the 'regular' call to JS_ReadStructuredClone for stuff like #structureddeserialize.

I guess the big question is where to put that StructuredCloneHolder? For example with PostMessage, currently we pass around a Vec<u8> which corresponds to the buffer in a StructuredCloneData. Could we instead pass around a StructuredCloneHolder containing that data, as well as keeping the DomRoot alive(not sure if that is Send and so on)? Or would we need to store the DomRoot elsewhere and access it on the other side of post message when reading? Would we need to keep the rooted dom alive after a successful read on the other side(I guess so, since it will still be used in JS at that point)?