From 06b04ec4ab5f0366fa20b286588c63f74141ea11 Mon Sep 17 00:00:00 2001 From: "jiayl@webrtc.org" Date: Thu, 24 Jul 2014 20:41:20 +0000 Subject: [PATCH] Fix a crash in statscollector.cc caused by invoking methods on the worker thread which destroys the Transport. BUG=3579 R=wu@webrtc.org Review URL: https://webrtc-codereview.appspot.com/17999004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@6776 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/app/webrtc/statscollector.cc | 26 ++++++++++++++-------- talk/app/webrtc/statscollector_unittest.cc | 18 +++++++-------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/talk/app/webrtc/statscollector.cc b/talk/app/webrtc/statscollector.cc index b165841aa0..94586fda7c 100644 --- a/talk/app/webrtc/statscollector.cc +++ b/talk/app/webrtc/statscollector.cc @@ -840,19 +840,27 @@ void StatsCollector::ExtractSessionInfo() { // Attempt to get a copy of the certificates from the transport and // expose them in stats reports. All channels in a transport share the // same local and remote certificates. + // + // Note that Transport::GetIdentity and Transport::GetRemoteCertificate + // invoke method calls on the worker thread and block this thread, but + // messages are still processed on this thread, which may blow way the + // existing transports. So we cannot reuse |transport| after these calls. std::string local_cert_report_id, remote_cert_report_id; + cricket::Transport* transport = session_->GetTransport(transport_iter->second.content_name); - if (transport) { - talk_base::scoped_ptr identity; - if (transport->GetIdentity(identity.accept())) - local_cert_report_id = AddCertificateReports( - &(identity->certificate())); - - talk_base::scoped_ptr cert; - if (transport->GetRemoteCertificate(cert.accept())) - remote_cert_report_id = AddCertificateReports(cert.get()); + talk_base::scoped_ptr identity; + if (transport && transport->GetIdentity(identity.accept())) { + local_cert_report_id = + AddCertificateReports(&(identity->certificate())); } + + transport = session_->GetTransport(transport_iter->second.content_name); + talk_base::scoped_ptr cert; + if (transport && transport->GetRemoteCertificate(cert.accept())) { + remote_cert_report_id = AddCertificateReports(cert.get()); + } + for (cricket::TransportChannelStatsList::iterator channel_iter = transport_iter->second.channel_stats.begin(); channel_iter != transport_iter->second.channel_stats.end(); diff --git a/talk/app/webrtc/statscollector_unittest.cc b/talk/app/webrtc/statscollector_unittest.cc index fc0ca0ed79..72ba111d54 100644 --- a/talk/app/webrtc/statscollector_unittest.cc +++ b/talk/app/webrtc/statscollector_unittest.cc @@ -614,7 +614,7 @@ class StatsCollectorTest : public testing::Test { // Configure MockWebRtcSession EXPECT_CALL(session_, GetTransport(transport_stats.content_name)) - .WillOnce(Return(transport.get())); + .WillRepeatedly(Return(transport.get())); EXPECT_CALL(session_, GetStats(_)) .WillOnce(DoAll(SetArgPointee<0>(session_stats), Return(true))); @@ -855,7 +855,7 @@ TEST_F(StatsCollectorTest, TransportObjectLinkedFromSsrcObject) { webrtc::StatsCollector stats(&session_); // Implementation under test. // Ignore unused callback (logspam). EXPECT_CALL(session_, GetTransport(_)) - .WillOnce(Return(static_cast(NULL))); + .WillRepeatedly(Return(static_cast(NULL))); MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); // The content_name known by the video channel. const std::string kVcName("vcname"); @@ -927,7 +927,7 @@ TEST_F(StatsCollectorTest, RemoteSsrcInfoIsPresent) { webrtc::StatsCollector stats(&session_); // Implementation under test. // Ignore unused callback (logspam). EXPECT_CALL(session_, GetTransport(_)) - .WillOnce(Return(static_cast(NULL))); + .WillRepeatedly(Return(static_cast(NULL))); MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); // The content_name known by the video channel. const std::string kVcName("vcname"); @@ -1072,7 +1072,7 @@ TEST_F(StatsCollectorTest, NoTransport) { // Configure MockWebRtcSession EXPECT_CALL(session_, GetTransport(transport_stats.content_name)) - .WillOnce(ReturnNull()); + .WillRepeatedly(ReturnNull()); EXPECT_CALL(session_, GetStats(_)) .WillOnce(DoAll(SetArgPointee<0>(session_stats), Return(true))); @@ -1125,7 +1125,7 @@ TEST_F(StatsCollectorTest, NoCertificates) { // Configure MockWebRtcSession EXPECT_CALL(session_, GetTransport(transport_stats.content_name)) - .WillOnce(Return(transport.get())); + .WillRepeatedly(Return(transport.get())); EXPECT_CALL(session_, GetStats(_)) .WillOnce(DoAll(SetArgPointee<0>(session_stats), Return(true))); @@ -1217,7 +1217,7 @@ TEST_F(StatsCollectorTest, GetStatsFromLocalAudioTrack) { webrtc::StatsCollector stats(&session_); // Implementation under test. // Ignore unused callback (logspam). EXPECT_CALL(session_, GetTransport(_)) - .WillOnce(Return(static_cast(NULL))); + .WillRepeatedly(Return(static_cast(NULL))); MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The content_name known by the voice channel. @@ -1250,7 +1250,7 @@ TEST_F(StatsCollectorTest, GetStatsFromRemoteStream) { webrtc::StatsCollector stats(&session_); // Implementation under test. // Ignore unused callback (logspam). EXPECT_CALL(session_, GetTransport(_)) - .WillOnce(Return(static_cast(NULL))); + .WillRepeatedly(Return(static_cast(NULL))); MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The content_name known by the voice channel. const std::string kVcName("vcname"); @@ -1276,7 +1276,7 @@ TEST_F(StatsCollectorTest, GetStatsAfterRemoveAudioStream) { webrtc::StatsCollector stats(&session_); // Implementation under test. // Ignore unused callback (logspam). EXPECT_CALL(session_, GetTransport(_)) - .WillOnce(Return(static_cast(NULL))); + .WillRepeatedly(Return(static_cast(NULL))); MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The content_name known by the voice channel. const std::string kVcName("vcname"); @@ -1333,7 +1333,7 @@ TEST_F(StatsCollectorTest, LocalAndRemoteTracksWithSameSsrc) { webrtc::StatsCollector stats(&session_); // Implementation under test. // Ignore unused callback (logspam). EXPECT_CALL(session_, GetTransport(_)) - .WillOnce(Return(static_cast(NULL))); + .WillRepeatedly(Return(static_cast(NULL))); MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The content_name known by the voice channel. const std::string kVcName("vcname");