Skip to content

Commit f1966ca

Browse files
committed
fix: dispatch transport calls to network thread instead of worker thread
WebRTC's SctpTransport, DtlsTransport, and IceTransport are all constructed on the network thread and assert owner_thread_->IsCurrent() (or creator_thread_->IsCurrent()) on methods like dtls_transport(), RegisterObserver(), UnregisterObserver(), and internal(). See the RTC_DCHECK_RUN_ON assertions in the WebRTC source: - pc/sctp_transport.cc lines 59, 66, 105 - pc/dtls_transport.cc lines 55, 61 - pc/ice_transport.cc line 27 The node-webrtc code was dispatching these calls to WorkerThread, which is the wrong thread and causes a crash in debug builds: Fatal error in: pc/sctp_transport.cc, line 105 Check failed: (owner_thread_)->IsCurrent() Additionally, PeerConnection::GetSctpTransport() requires the network thread (pc/peer_connection.cc line 1817). Change all call sites to use NetworkThread(). For Stop() methods that may be called from network-thread callbacks (e.g. OnStateChange), use IsCurrent() to avoid a redundant BlockingCall.
1 parent 82b1335 commit f1966ca

5 files changed

Lines changed: 25 additions & 11 deletions

File tree

src/interfaces/rtc_data_channel.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ void RTCDataChannel::OnPeerConnectionClosed() {
125125
}
126126

127127
void RTCDataChannel::OnStateChange() {
128+
if (_jingleDataChannel == nullptr) {
129+
return;
130+
}
128131
auto state = _jingleDataChannel->state();
129132
if (state == webrtc::DataChannelInterface::kClosed) {
130133
CleanupInternals();

src/interfaces/rtc_dtls_transport.cc

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ RTCDtlsTransport::RTCDtlsTransport(const Napi::CallbackInfo &info)
7676
// NOTE(mroberts): Ensure we create this.
7777
_transport_wrap.GetOrCreate(_factory, _transport->ice_transport());
7878

79-
_factory->WorkerThread()->BlockingCall([this]() {
79+
_factory->NetworkThread()->BlockingCall([this]() {
8080
_transport->RegisterObserver(this);
8181
auto information = _transport->Information();
8282
_state = information.state();
@@ -94,7 +94,12 @@ RTCDtlsTransport::~RTCDtlsTransport() {
9494
}
9595

9696
void RTCDtlsTransport::Stop() {
97-
_transport->UnregisterObserver();
97+
if (_factory->NetworkThread()->IsCurrent()) {
98+
_transport->UnregisterObserver();
99+
} else {
100+
_factory->NetworkThread()->BlockingCall(
101+
[this]() { _transport->UnregisterObserver(); });
102+
}
98103
auto ice_transport = _transport_wrap.Get(_transport->ice_transport());
99104
if (ice_transport) {
100105
ice_transport->OnRTCDtlsTransportStopped();

src/interfaces/rtc_ice_transport.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ RTCIceTransport::RTCIceTransport(const Napi::CallbackInfo &info)
3838

3939
_transport = std::move(transport);
4040

41-
_factory->WorkerThread()->BlockingCall([this]() {
41+
_factory->NetworkThread()->BlockingCall([this]() {
4242
auto internal = _transport->internal();
4343
if (internal) {
4444
internal->SignalIceTransportStateChanged.connect(

src/interfaces/rtc_peer_connection.cc

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -938,12 +938,13 @@ RTCPeerConnection::GetPendingRemoteDescription(const Napi::CallbackInfo &info) {
938938
}
939939

940940
Napi::Value RTCPeerConnection::GetSctp(const Napi::CallbackInfo &info) {
941-
return _jinglePeerConnection && _jinglePeerConnection->GetSctpTransport()
942-
? _transport_wrap
943-
.GetOrCreate(_factory,
944-
_jinglePeerConnection->GetSctpTransport())
945-
->Value()
946-
: info.Env().Null();
941+
auto transport = _jinglePeerConnection
942+
? _factory->NetworkThread()->BlockingCall([this]() {
943+
return _jinglePeerConnection->GetSctpTransport();
944+
})
945+
: nullptr;
946+
return transport ? _transport_wrap.GetOrCreate(_factory, transport)->Value()
947+
: info.Env().Null();
947948
}
948949

949950
Napi::Value

src/interfaces/rtc_sctp_transport.cc

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ RTCSctpTransport::RTCSctpTransport(const Napi::CallbackInfo &info)
3838

3939
_transport = std::move(transport);
4040

41-
_factory->WorkerThread()->BlockingCall([this]() {
41+
_factory->NetworkThread()->BlockingCall([this]() {
4242
_dtls_transport = _transport->dtls_transport();
4343
_transport->RegisterObserver(this);
4444
});
@@ -54,7 +54,12 @@ RTCSctpTransport::RTCSctpTransport(const Napi::CallbackInfo &info)
5454
RTCSctpTransport::~RTCSctpTransport() { wrap()->Release(this); }
5555

5656
void RTCSctpTransport::Stop() {
57-
_transport->UnregisterObserver();
57+
if (_factory->NetworkThread()->IsCurrent()) {
58+
_transport->UnregisterObserver();
59+
} else {
60+
_factory->NetworkThread()->BlockingCall(
61+
[this]() { _transport->UnregisterObserver(); });
62+
}
5863
AsyncObjectWrapWithLoop<RTCSctpTransport>::Stop();
5964
}
6065

0 commit comments

Comments
 (0)