Bug 22397 – Worker threads are not destroyed if running a tight loop

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

[Alexey Proskuryakov]

Worker threads need to be destroyed when their Worker object is destroyed. Currently, this doesn't happen if the thread in running a JS loop, because the event loop in not entered in this case.

[Alexey Proskuryakov]

Created attachment 25345 [details]
proposed fix

[Alexey Proskuryakov]

Created attachment 25347 [details]
proposed fix

Removed access to possibly already destroyed "this" object.

[Darin Adler]

Comment on attachment 25347 [details]
proposed fix

> +    m_globalData->interpreter->setTimeoutTime(1);

What does this "1" mean? Is it a magic constant of some sort? 1 second perhaps?

> +    // The thread object may be already destroyed from notification now, don't try to access "this".

I don't think this comment makes it clear enough that it was the assignment m_workerContext = 0 that could trigger the notification. I had to study the code for a while to understand why the previous line could access "this" without a problem, until I realized that it was the previous line that was the trigger.

> +    // FIXME: rudely killing the thread won't work when we allow nested workers, because they will try to post notifications of their destruction.
> +    m_messageQueue.kill();

Please capitalize "rudely".

r=me

[Alexey Proskuryakov]

(In reply to comment #3)
> (From update of attachment 25347 [details] [review])
> > +    m_globalData->interpreter->setTimeoutTime(1);
> 
> What does this "1" mean? Is it a magic constant of some sort? 1 second perhaps?

This is 1 millisecond, which is the minimal value that can be passed to this function (0 is magic, and means no timeout).

[Alexey Proskuryakov]

Committed revision 38689. Re-worded the comments a bit for clarity.