Bug 21769 – MessagePort should be GC protected if there are messages to be delivered

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Alexey Proskuryakov]

Even if a message port and its entangled port both become inaccessible from JS, outstanding messages should be delivered, as there may be listeners registered.

This is kinda obvious, as it is too easy to write JS code that fails randomly if GC kicks in just in time for messages to be lost. But HTML5 was just changed to say this.

Patch forthcoming.

[Alexey Proskuryakov]

Created attachment 24538 [details]
proposed fix

[Darin Adler]

Comment on attachment 24538 [details]
proposed fix

+        // Warning: using the below methods is risky, as the queue state may change by the time they return. Extra caution is advised.

I like what this comment is warning about, but I'm not fond of "risky" and "caution" when describing how to use functions. And "queue state" is also pretty vague. It would be better if this was concrete about what the consideration is. I think that "be careful" doesn't help as much as specifics. Maybe a comment more like "the result of isEmpty() is only useful if there's some other guarantee that no one is going to add messages to the queue on another thread" would be one good example. I think that's better than talking about risk and caution. The comment in MessagePort::hasPendingActivity is nice and clear on this point.

r=me

[Alexey Proskuryakov]

(In reply to comment #2)
> I like what this comment is warning about, but I'm not fond of "risky" and
> "caution" when describing how to use functions.

OK. I changed the warning for isEmpty(), but didn't find a good way to specifically warn about tryGetMessage()/killed(). Given that those are normally used for polling, maybe it's not important to warn about these.