Summary: | Worker threads are not destroyed if running a tight loop | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||
Component: | WebCore JavaScript | Assignee: | Alexey Proskuryakov <ap> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Alexey Proskuryakov
2008-11-21 02:48:50 PST
Created attachment 25345 [details]
proposed fix
Created attachment 25347 [details]
proposed fix
Removed access to possibly already destroyed "this" object.
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 (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). Committed revision 38689. Re-worded the comments a bit for clarity. |