admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert of Free SCTP data channels asynchronously in PeerConnection. (patchset #3 id:40001 of https://codereview.webrtc.org/1492383002/ )

Browse Source 
Reason for revert:
Breaks WebrtcTransportTest.DataStream, due to different rtc::Thread implementation on Chromium.

Original issue's description:
> Free SCTP data channels asynchronously in PeerConnection.
>
> This is needed so that the data channel isn't deleted while one of its
> own methods is on the call stack.
>
> BUG=565048
>
> Committed: https://crrev.com/386869247f28e72a00307a1b5c92465eea343ad2
> Cr-Commit-Position: refs/heads/master@{#10923}

TBR=pthatcher@webrtc.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=565048

Review URL: https://codereview.webrtc.org/1513143003

Cr-Commit-Position: refs/heads/master@{#10977}
This commit is contained in:
deadbeef 2015-12-10 11:17:42 -08:00 committed by Commit bot
parent 61a90f94b6
commit a1f567ae90
2 changed files with 0 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
talk/app/webrtc/peerconnection.cc
View File

@ -107,7 +107,6 @@ enum {
MSG_SET_SESSIONDESCRIPTION_FAILED,
MSG_CREATE_SESSIONDESCRIPTION_FAILED,
MSG_GETSTATS,
MSG_DELETE,
};
struct SetSessionDescriptionMsg : public rtc::MessageData {
@ -598,8 +597,6 @@ PeerConnection::PeerConnection(PeerConnectionFactory* factory)
PeerConnection::~PeerConnection() {
TRACE_EVENT0("webrtc", "PeerConnection::~PeerConnection");
RTC_DCHECK(signaling_thread()->IsCurrent());
// Finish any pending deletions.
signaling_thread()->Clear(this, MSG_DELETE, nullptr);
// Need to detach RTP senders/receivers from WebRtcSession,
// since it's about to be destroyed.
for (const auto& sender : senders_) {
@ -1332,10 +1329,6 @@ void PeerConnection::OnMessage(rtc::Message* msg) {
delete param;
break;
}
case MSG_DELETE: {
delete msg->pdata;
break;
}
default:
RTC_DCHECK(false && "Not implemented");
break;
@ -1915,11 +1908,6 @@ void PeerConnection::OnSctpDataChannelClosed(DataChannel* 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.
signaling_thread()->Post(
this, MSG_DELETE,
new rtc::TypedMessageData<rtc::scoped_refptr<DataChannel>>(channel));
sctp_data_channels_.erase(it);
return;
}

27
talk/app/webrtc/peerconnectionendtoend_unittest.cc
View File

@ -402,30 +402,3 @@ 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<DataChannelInterface> 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);
}