Bug 170492 – WebRTC aborts when trying to sleep on a wrong thread

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

[Alexey Proskuryakov]

Saw a crash on webrtc/datachannel/bufferedAmountLowThreshold.html:

Thread 5 Crashed:: Thread 0x0x7f9697406b
0   libsystem_kernel.dylib        	0x00007fffcfe31dd6 __pthread_kill + 10
1   libsystem_pthread.dylib       	0x00007fffcff1d787 pthread_kill + 90
2   libsystem_c.dylib             	0x00007fffcfd97420 abort + 129
3   libwebrtc.dylib               	0x000000010ca65eb4 rtc::FatalMessage::~FatalMessage() + 452
4   libwebrtc.dylib               	0x000000010ca65ed5 rtc::FatalMessage::~FatalMessage() + 21
5   libwebrtc.dylib               	0x000000010d03c8e1 rtc::Thread::AssertBlockingIsAllowedOnCurrentThread() + 209 (thread.cc:298)
6   libwebrtc.dylib               	0x000000010d03c6e3 rtc::Thread::SleepMs(int) + 19 (thread.cc:174)
7   libwebrtc.dylib               	0x000000010d18bc21 cricket::SctpTransport::UsrSctpWrapper::UninitializeUsrSctp() + 241 (sctptransport.cc:264)
8   libwebrtc.dylib               	0x000000010d1844a9 cricket::SctpTransport::UsrSctpWrapper::DecrementUsrSctpUsageCount() + 57 (sctptransport.cc:281)
9   libwebrtc.dylib               	0x000000010d17fe92 cricket::SctpTransport::CloseSctpSocket() + 306 (sctptransport.cc:755)
10  libwebrtc.dylib               	0x000000010d17fce5 cricket::SctpTransport::~SctpTransport() + 53 (sctptransport.cc:401)
11  libwebrtc.dylib               	0x000000010d17ff35 cricket::SctpTransport::~SctpTransport() + 21 (sctptransport.cc:401)
12  libwebrtc.dylib               	0x000000010d17ff79 cricket::SctpTransport::~SctpTransport() + 25 (sctptransport.cc:398)
13  libwebrtc.dylib               	0x000000010cdbd908 webrtc::WebRtcSession::DestroySctpTransport_n() + 440 (memory:2724)
14  libwebrtc.dylib               	0x000000010cddac94 void rtc::MethodFunctor<webrtc::WebRtcSession, void (webrtc::WebRtcSession::*)(), void>::CallMethod<>(rtc::sequence<>) const + 100 (bind.h:164)
15  libwebrtc.dylib               	0x000000010cddac25 rtc::MethodFunctor<webrtc::WebRtcSession, void (webrtc::WebRtcSession::*)(), void>::operator()() const + 21 (bind.h:155)
16  libwebrtc.dylib               	0x000000010cddac00 rtc::FunctorMessageHandler<void, rtc::MethodFunctor<webrtc::WebRtcSession, void (webrtc::WebRtcSession::*)(), void> >::OnMessage(rtc::Message*) + 32 (messagehandler.h:66)
17  libwebrtc.dylib               	0x000000010d03dd9e rtc::Thread::ReceiveSendsFromThread(rtc::Thread const*) + 126 (thread.cc:425)
18  libwebrtc.dylib               	0x000000010d03de29 rtc::Thread::ReceiveSends() + 25 (thread.cc:409)
19  libwebrtc.dylib               	0x000000010cc21bfd rtc::MessageQueue::Get(rtc::Message*, int, bool) + 189 (messagequeue.cc:296)
20  libwebrtc.dylib               	0x000000010d03d19c rtc::Thread::ProcessMessages(int) + 140 (thread.cc:494)

[Alexey Proskuryakov]

Created attachment 306251 [details]
crash log

[Radar WebKit Bug Importer]

<rdar://problem/31446377>

[youenn fablet]

(In reply to Radar WebKit Bug Importer from comment #2)
> <rdar://problem/31446377>

Probably the same as 31375728

[youenn fablet]

Created attachment 306252 [details]
Patch

[Alexey Proskuryakov]

Is this really the right thing to do? Blocking a network thread is generally not advisable, especially in real time communication.

[Build Bot]

Comment on attachment 306252 [details]
Patch

Attachment 306252 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3475974

New failing tests:
fast/mediastream/getUserMedia-webaudio.html

[Build Bot]

Created attachment 306263 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Eric Carlson]

Comment on attachment 306252 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=306252&action=review

> Source/ThirdParty/libwebrtc/Source/webrtc/media/sctp/sctptransport.cc:262
>        if (usrsctp_finish() == 0) {
>          return;

It OK to leave the thread accepting blocking calls?

[youenn fablet]

> Is this really the right thing to do? Blocking a network thread is generally
> not advisable, especially in real time communication.

Agreed, that is far from ideal.

Moving to asynchronous finishing might be difficult in the case where data channels might get created when others are deleted.

We could reduce the blocking case by calling IncrementUsrSctpUsageCount() ourselves so that finish usrsctp would only happen at the end of the process and where blocking is fine. But the issue is that we would not free memory until that time.

All in all, that is how libwebrtc is implemented and working right now.
So I am not sure this is worth the effort to maintain something different ourselves.

> Is it OK to leave the thread accepting blocking calls?

Right, SetAllowBlockingCalls should be moved closer to Sleep.

[youenn fablet]

Created attachment 306400 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 306400 [details]
Patch

Clearing flags on attachment: 306400

Committed r215049: <http://trac.webkit.org/changeset/215049>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.