generic-library/webrtc
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Revert "PeerConnectionFactory: delay deletion of owned threads."

Browse Source 
This reverts r5933 because it broke
http://build.chromium.org/p/client.webrtc/builders/Win64%20Release/builds/1598

BUG=3100

Review URL: https://webrtc-codereview.appspot.com/12159004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5935 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
fischman@webrtc.org 2014-04-17 22:54:30 +00:00
parent 1a87f529a2
commit 29540b1879
2 changed files with 2 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
talk/app/webrtc/peerconnectionfactory.cc
View File

@ -183,33 +183,12 @@ PeerConnectionFactory::PeerConnectionFactory(
// ASSERT(default_adm != NULL); // ASSERT(default_adm != NULL);
} }
// Deletes |thread| if it is not the current thread, else causes it to
// exit and delete itself after completing the currently processing
// message.
//
// NOTE: this is required because:
// 1) PeerConnection holds a ref on PeerConnectionFactory; and
// 2) PeerConnectionFactory may own the signaling & worker threads; and
// 3) PeerConnection is always destroyed on the signaling thread.
// As a result, if the last ref on PeerConnectionFactory is held by
// PeerConnection, a naive "delete signaling_thread_;" in
// ~PeerConnectionFactory() would result in deadlock. See
// https://code.google.com/p/webrtc/issues/detail?id=3100 for history.
static void DeleteOrRelease(talk_base::Thread* thread) {
if (thread->IsCurrent()) {
thread->Release(); // Causes thread to delete itself after Quit().
thread->Quit();
} else {
delete thread; // Calls thread->Stop() implicitly.
}
}
PeerConnectionFactory::~PeerConnectionFactory() { PeerConnectionFactory::~PeerConnectionFactory() {
signaling_thread_->Clear(this); signaling_thread_->Clear(this);
signaling_thread_->Send(this, MSG_TERMINATE_FACTORY); signaling_thread_->Send(this, MSG_TERMINATE_FACTORY);
if (owns_ptrs_) { if (owns_ptrs_) {
DeleteOrRelease(signaling_thread_); delete signaling_thread_;
DeleteOrRelease(worker_thread_); delete worker_thread_;
} }
} }

21
talk/app/webrtc/peerconnectionfactory_unittest.cc
View File

@ -340,24 +340,3 @@ TEST_F(PeerConnectionFactoryTest, LocalRendering) {
EXPECT_TRUE(capturer->CaptureFrame()); EXPECT_TRUE(capturer->CaptureFrame());
EXPECT_EQ(2, local_renderer.num_rendered_frames()); EXPECT_EQ(2, local_renderer.num_rendered_frames());
} }
// Test that no deadlock or ASSERT triggers on releasing the last
// reference to a PeerConnectionFactory (regression test for
// https://code.google.com/p/webrtc/issues/detail?id=3100).
TEST(PeerConnectionFactory2Test, ThreadTeardown) {
talk_base::scoped_refptr<PeerConnectionFactoryInterface> factory(
webrtc::CreatePeerConnectionFactory());
NullPeerConnectionObserver observer;
talk_base::scoped_refptr<PeerConnectionInterface> pc(
factory->CreatePeerConnection(
webrtc::PeerConnectionInterface::IceServers(),
NULL,
NULL,
&observer));
factory = NULL;
// Now |pc| holds the last ref to the factory (and thus its
// threads). If the next line, which causes |pc| to be freed,
// doesn't ASSERT (in Debug) or deadlock (in Release) then the test
// is successful.
pc = NULL;
}