diff --git a/talk/app/webrtc/peerconnection.cc b/talk/app/webrtc/peerconnection.cc index 83e29191c8..83380e17b5 100644 --- a/talk/app/webrtc/peerconnection.cc +++ b/talk/app/webrtc/peerconnection.cc @@ -108,6 +108,7 @@ enum { MSG_SET_SESSIONDESCRIPTION_FAILED, MSG_CREATE_SESSIONDESCRIPTION_FAILED, MSG_GETSTATS, + MSG_FREE_DATACHANNELS, }; struct SetSessionDescriptionMsg : public rtc::MessageData { @@ -1332,6 +1333,10 @@ void PeerConnection::OnMessage(rtc::Message* msg) { delete param; break; } + case MSG_FREE_DATACHANNELS: { + sctp_data_channels_to_free_.clear(); + break; + } default: RTC_DCHECK(false && "Not implemented"); break; @@ -1905,13 +1910,18 @@ void PeerConnection::AllocateSctpSids(rtc::SSLRole role) { } void PeerConnection::OnSctpDataChannelClosed(DataChannel* channel) { + RTC_DCHECK(signaling_thread()->IsCurrent()); for (auto it = sctp_data_channels_.begin(); it != sctp_data_channels_.end(); ++it) { if (it->get() == channel) { if (channel->id() >= 0) { sid_allocator_.ReleaseSid(channel->id()); } + // Since this method is triggered by a signal from the DataChannel, + // we can't free it directly here; we need to free it asynchronously. + sctp_data_channels_to_free_.push_back(*it); sctp_data_channels_.erase(it); + signaling_thread()->Post(this, MSG_FREE_DATACHANNELS, nullptr); return; } } diff --git a/talk/app/webrtc/peerconnection.h b/talk/app/webrtc/peerconnection.h index 12f8d1bd4b..9cb3635690 100644 --- a/talk/app/webrtc/peerconnection.h +++ b/talk/app/webrtc/peerconnection.h @@ -374,6 +374,7 @@ class PeerConnection : public PeerConnectionInterface, // label -> DataChannel std::map> rtp_data_channels_; std::vector> sctp_data_channels_; + std::vector> sctp_data_channels_to_free_; bool remote_peer_supports_msid_ = false; rtc::scoped_ptr remote_stream_factory_; diff --git a/talk/app/webrtc/peerconnectionendtoend_unittest.cc b/talk/app/webrtc/peerconnectionendtoend_unittest.cc index 1d7bb9211e..aff117e1bb 100644 --- a/talk/app/webrtc/peerconnectionendtoend_unittest.cc +++ b/talk/app/webrtc/peerconnectionendtoend_unittest.cc @@ -402,3 +402,30 @@ TEST_F(PeerConnectionEndToEndTest, CloseDataChannels(caller_dc, callee_signaled_data_channels_, 1); } + +// This tests that if a data channel is closed remotely while not referenced +// by the application (meaning only the PeerConnection contributes to its +// reference count), no memory access violation will occur. +// See: https://code.google.com/p/chromium/issues/detail?id=565048 +TEST_F(PeerConnectionEndToEndTest, CloseDataChannelRemotelyWhileNotReferenced) { + MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); + + CreatePcs(); + + webrtc::DataChannelInit init; + rtc::scoped_refptr caller_dc( + caller_->CreateDataChannel("data", init)); + + Negotiate(); + WaitForConnection(); + + WaitForDataChannelsToOpen(caller_dc, callee_signaled_data_channels_, 0); + // This removes the reference to the remote data channel that we hold. + callee_signaled_data_channels_.clear(); + caller_dc->Close(); + EXPECT_EQ_WAIT(DataChannelInterface::kClosed, caller_dc->state(), kMaxWait); + + // Wait for a bit longer so the remote data channel will receive the + // close message and be destroyed. + rtc::Thread::Current()->ProcessMessages(100); +}