Bug 187702 – Make sure LibWebRTCMediaEndpoint is always destroyed on the main thread

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Chris Dumez]

Make sure LibWebRTCMediaEndpoint is always destroyed on the main thread since it has a Timer data member.

[Chris Dumez]

Created attachment 345098 [details]
Patch

[youenn fablet]

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?

[Chris Dumez]

(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()));

[Chris Dumez]

(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).

[WebKit Commit Bot]

Comment on attachment 345098 [details]
Patch

Clearing flags on attachment: 345098

Committed r233857: <https://trac.webkit.org/changeset/233857>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/42249803>