1476518 - Cycle collect inter-thread entangled MessagePort (connection) references

Status: NEW

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Karl Tomlinson (:karlt)]

AudioWorklet introduces a use case where the number of MessageChannels with short lifetime used over time is expected to often be unbounded.  The MessagePorts are initially on separate threads.

https://webaudio.github.io/web-audio-api/#instantiation-of-AudioWorkletNode-and-AudioWorkletProcessor

Comment 1

[Karl Tomlinson (:karlt)]

Note that one thread is typically a realtime audio thread, and so inter-thread
cycle collection will need to be incremental rather than stop-the-world.

Comment 2

[Andrew McCreight [:mccr8]]

What do you mean by "short lifetime"? The cycle collector only runs around every 10 seconds.

Are the MessagePorts all on a single thread, or on multiple threads? The cycle collector only works on a single worker (or the main thread).

Comment 3

[Andrea Marchesini [:baku]]

MessagePorts, from a CC point of view, are single thread objects. When a port is sent to a different thread/process, a new MessagePort object is created in the destination nsIGlobalObject. It doesn't require multiple-thread CC: it works perfectly in workers and it should work correctly in worklet as well.

Cycle Collector in worklet will take care of MessagePort in the worklet thread.

Note that 2 entangled MessagePorts keep themselves alive. The entangling must be broken by a port.close().

Comment 4

[Karl Tomlinson (:karlt)]

Thank you Andrew and Andrea, for your interest.
I expect some guidance here would be very helpful.

(In reply to Andrew McCreight [:mccr8] from comment #2)
> What do you mean by "short lifetime"? The cycle collector only runs around
> every 10 seconds.

I mean merely less than the lifetime of the worklet.
The rate at which cycles are collected is not important, as long as they
collected at some point.

> Are the MessagePorts all on a single thread, or on multiple threads?

Multiple.
Entangled MessagePorts are on different threads.

> The cycle collector only works on a single worker (or the main thread).

Yes, what we usually refer to as the cycle collector would not be sufficient
on its own (or as is) to collect entangled MessagePorts.

This bug is about adding what is necessary.

Comment 5

[Karl Tomlinson (:karlt)]

(In reply to Andrea Marchesini [:baku] from comment #3)
> MessagePorts, from a CC point of view, are single thread objects. When a
> port is sent to a different thread/process, a new MessagePort object is
> created in the destination nsIGlobalObject. It doesn't require
> multiple-thread CC: it works perfectly in workers and it should work
> correctly in worklet as well.

I assume that works perfectly well for workers either because entangled
MessagePorts are used for the lifetime of the worker, or not many are
typically used, or web developers know to explicitly break the cycles with
close().

> Note that 2 entangled MessagePorts keep themselves alive.

Thank you for directing me to the appropriate language.
I'll update the title to clarify.
That is what this bug is about.

> The entangling must be broken by a port.close().

If the entangling is not broken, but there are no rooted references to either
port (with inter-thread meaning of "rooted"), then the entangled pair is
garbage, which may be collected.  There is no need to keep it alive because it
is not observable.

https://html.spec.whatwg.org/multipage/web-messaging.html#ports-and-garbage-collection
has a related note:
"Authors are strongly encouraged to explicitly close MessagePort objects to
disentangle them, so that their resources can be recollected. Creating many
MessagePort objects and discarding them without closing them can lead to high
transient memory usage since garbage collection is not necessarily performed
promptly, especially for MessagePorts where garbage collection can involve
cross-process coordination."

The examples in the Web Audio API do not explicitly disentangle
(e.g. https://webaudio.github.io/web-audio-api/#vu-meter-mode)
and there is no reason why they should need to do so, except to work around
lack of inter-thread cycle collection in some browsers.

Comment 6

[Andrew McCreight [:mccr8]]

This is a difficult problem to solve in general because potentially you can have something like A -> Pair1 -> B -> Pair2 -> A, where Pair1 and Pair2 are each a pair of these message ports, and A and B are various clumps of JS and DOM objects, where A and B are on different threads.

(In reply to Karl Tomlinson (:karlt) from comment #5)
> and there is no reason why they should need to do so, except to work around
> lack of inter-thread cycle collection in some browsers.

Are there any browsers that can do this?

Comment 7

[Andrew McCreight [:mccr8]]

(In reply to Karl Tomlinson (:karlt) from comment #4)
> I mean merely less than the lifetime of the worklet.
> The rate at which cycles are collected is not important, as long as they
> collected at some point.

Alright. I was just asking because if you are eg allocating 10000 a frame or something and hoping they'll be cleaned up by the next frame, that's going to require a different solution than just collecting them every 10 or so seconds.

Comment 8

[Karl Tomlinson (:karlt)]

(In reply to Andrew McCreight [:mccr8] from comment #6)
> This is a difficult problem to solve in general because potentially you can
> have something like A -> Pair1 -> B -> Pair2 -> A, where Pair1 and Pair2 are
> each a pair of these message ports, and A and B are various clumps of JS and
> DOM objects, where A and B are on different threads.

I'm expecting a similar situation A -> Pair1 -> B -> Pair1 -> A, where A and B
are clumps of JS and C++ objects, to be common, but not necessarily
ubiquitous.

> (In reply to Karl Tomlinson (:karlt) from comment #5)
> > and there is no reason why they should need to do so, except to work around
> > lack of inter-thread cycle collection in some browsers.
> 
> Are there any browsers that can do this?

https://bugs.chromium.org/p/chromium/issues/detail?id=798855 would suggest
that Blink does not.

https://bugs.webkit.org/attachment.cgi?id=331857&action=prettypatch seems to
involve some level of support for Webkit, but I'm not clear how complete it
is.

The Blink code still looks somewhat like an old version of the Webkit code.

If Gecko's architecture is going to need to be completely redesigned in order
to accommodate this, then I guess we can hide behind Blink and write docs
telling developers to manually clean up, assuming Google don't fix their leaks
first.

The A -> Pair1 -> B situation, with only one onmessage handler, may be the
slightly more common.  I expect that would be easy to handle.  (Comment 3 and
maybe bug 1177776 imply we do not currently.)  If a full solution is not
practical, then I'd like to at least check we don't leak that.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

I would be very surprised if webkit could break any cross-threads cycles given that it doesn't really have any generic setup for breaking cycles even in single thread case.

Comment 10

[Andrew McCreight [:mccr8]]

If you look at the WebKit commit in comment 8, it looks like they use some logic based on the state of the message port itself, not anything about whether it is referred to by other things. I'm not sure how much of their list we implement, if any.