Closed Bug 854739 Opened 7 years ago Closed 6 years ago

Suspending and resuming workers can lead to messages being delivered to the main thread out of order

Categories

(Core :: DOM: Workers, defect)

22 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: khuey, Assigned: khuey)

Details

Attachments

(1 file)

Because we only queue messages when we reach them while running the event queue, we can deliver messages out of order if we resume a worker while messages that were pending when it was suspended are still pending in the event queue.
Attached patch PatchSplinter Review
Assignee: nobody → khuey
Status: NEW → ASSIGNED
Attachment #743238 - Flags: review?(bent.mozilla)
Comment on attachment 743238 [details] [diff] [review]
Patch

Review of attachment 743238 [details] [diff] [review]:
-----------------------------------------------------------------

r=bholley on the JSContext stuff.
Attachment #743238 - Flags: review?(bobbyholley+bmo) → review+
Comment on attachment 743238 [details] [diff] [review]
Patch

Review of attachment 743238 [details] [diff] [review]:
-----------------------------------------------------------------

r=me.

::: dom/workers/WorkerPrivate.cpp
@@ +1404,5 @@
>  };
>  
> +class SynchronizeAndResumeRunnable : public nsRunnable
> +{
> +protected:

You just want private.

@@ +1406,5 @@
> +class SynchronizeAndResumeRunnable : public nsRunnable
> +{
> +protected:
> +  WorkerPrivate* mWorkerPrivate;
> +  nsCOMPtr<nsIScriptContext> mCx;

Is there a risk that the nsIScriptContext could be modified somehow between the time this runnable is queued and executed? What happens if the window gets torn down or something? Obviously you're holding the context alive but will it work as expected?
Attachment #743238 - Flags: review?(bent.mozilla) → review+
Kyle, you going to land this?
If the tree is ever open when I'm around, yes.
https://hg.mozilla.org/mozilla-central/rev/2550c1e294cd
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.