Summary: | Make sure LibWebRTCMediaEndpoint is always destroyed on the main thread | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||
Component: | Media | Assignee: | Chris Dumez <cdumez> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | achristensen, commit-queue, eric.carlson, rniwa, webkit-bug-importer, youennf | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Chris Dumez
2018-07-16 10:26:49 PDT
Created attachment 345098 [details]
Patch
Comment on attachment 345098 [details]
Patch
It seems safer with the patch.
I wonder whether this is an actual issue though.
When the pc goes away, LibWebRTCMediaEndpoint::close() is called which stops the timer.
Or is it needed to destroy the timer as well?
(In reply to youenn fablet from comment #2) > Comment on attachment 345098 [details] > Patch > > It seems safer with the patch. > I wonder whether this is an actual issue though. > When the pc goes away, LibWebRTCMediaEndpoint::close() is called which stops > the timer. > Or is it needed to destroy the timer as well? You'd still hit release assertions I believe, the Timer destructor calls stop() and stop() calls TimerBase::setNextFireTime() which has: RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get())); (In reply to Chris Dumez from comment #3) > (In reply to youenn fablet from comment #2) > > Comment on attachment 345098 [details] > > Patch > > > > It seems safer with the patch. > > I wonder whether this is an actual issue though. > > When the pc goes away, LibWebRTCMediaEndpoint::close() is called which stops > > the timer. > > Or is it needed to destroy the timer as well? > > You'd still hit release assertions I believe, the Timer destructor calls > stop() and stop() calls TimerBase::setNextFireTime() which has: > RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get())); This became a RELEASE_ASSERT() very recently in http://trac.webkit.org/r233821 to try and find cases where we're destroying timers on the wrong thread (As this could explain rdar://problem/33352721). Comment on attachment 345098 [details] Patch Clearing flags on attachment: 345098 Committed r233857: <https://trac.webkit.org/changeset/233857> All reviewed patches have been landed. Closing bug. |