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
diff --git a/talk/app/webrtc/statscollector.cc b/talk/app/webrtc/statscollector.cc
index b165841..94586fd 100644
--- a/talk/app/webrtc/statscollector.cc
+++ b/talk/app/webrtc/statscollector.cc
@@ -840,19 +840,27 @@
// 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<talk_base::SSLIdentity> identity;
- if (transport->GetIdentity(identity.accept()))
- local_cert_report_id = AddCertificateReports(
- &(identity->certificate()));
-
- talk_base::scoped_ptr<talk_base::SSLCertificate> cert;
- if (transport->GetRemoteCertificate(cert.accept()))
- remote_cert_report_id = AddCertificateReports(cert.get());
+ talk_base::scoped_ptr<talk_base::SSLIdentity> 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<talk_base::SSLCertificate> 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 fc0ca0e..72ba111 100644
--- a/talk/app/webrtc/statscollector_unittest.cc
+++ b/talk/app/webrtc/statscollector_unittest.cc
@@ -614,7 +614,7 @@
// 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 @@
webrtc::StatsCollector stats(&session_); // Implementation under test.
// Ignore unused callback (logspam).
EXPECT_CALL(session_, GetTransport(_))
- .WillOnce(Return(static_cast<cricket::Transport*>(NULL)));
+ .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
// The content_name known by the video channel.
const std::string kVcName("vcname");
@@ -927,7 +927,7 @@
webrtc::StatsCollector stats(&session_); // Implementation under test.
// Ignore unused callback (logspam).
EXPECT_CALL(session_, GetTransport(_))
- .WillOnce(Return(static_cast<cricket::Transport*>(NULL)));
+ .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
// The content_name known by the video channel.
const std::string kVcName("vcname");
@@ -1072,7 +1072,7 @@
// 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 @@
// 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 @@
webrtc::StatsCollector stats(&session_); // Implementation under test.
// Ignore unused callback (logspam).
EXPECT_CALL(session_, GetTransport(_))
- .WillOnce(Return(static_cast<cricket::Transport*>(NULL)));
+ .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
// The content_name known by the voice channel.
@@ -1250,7 +1250,7 @@
webrtc::StatsCollector stats(&session_); // Implementation under test.
// Ignore unused callback (logspam).
EXPECT_CALL(session_, GetTransport(_))
- .WillOnce(Return(static_cast<cricket::Transport*>(NULL)));
+ .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
// The content_name known by the voice channel.
const std::string kVcName("vcname");
@@ -1276,7 +1276,7 @@
webrtc::StatsCollector stats(&session_); // Implementation under test.
// Ignore unused callback (logspam).
EXPECT_CALL(session_, GetTransport(_))
- .WillOnce(Return(static_cast<cricket::Transport*>(NULL)));
+ .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
// The content_name known by the voice channel.
const std::string kVcName("vcname");
@@ -1333,7 +1333,7 @@
webrtc::StatsCollector stats(&session_); // Implementation under test.
// Ignore unused callback (logspam).
EXPECT_CALL(session_, GetTransport(_))
- .WillOnce(Return(static_cast<cricket::Transport*>(NULL)));
+ .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
// The content_name known by the voice channel.
const std::string kVcName("vcname");