Remove send and receive streams when destroyed.
Fixes crash where packets were sent to a receive stream that had been
destroyed but not removed from the ssrc mapping from call to receiver.
Added a repro case that reliably crashed before the fix.
BUG=
R=stefan@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/2161007
git-svn-id: http://webrtc.googlecode.com/svn/trunk@4681 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/video_engine/internal/video_call.cc b/webrtc/video_engine/internal/video_call.cc
index 97f606a..6e50395 100644
--- a/webrtc/video_engine/internal/video_call.cc
+++ b/webrtc/video_engine/internal/video_call.cc
@@ -95,11 +95,25 @@
SendStreamState* VideoCall::DestroySendStream(
webrtc::VideoSendStream* send_stream) {
- if (send_stream == NULL) {
- return NULL;
+ assert(send_stream != NULL);
+
+ VideoSendStream* send_stream_impl = NULL;
+ {
+ WriteLockScoped write_lock(*send_lock_);
+ for (std::map<uint32_t, VideoSendStream*>::iterator it =
+ send_ssrcs_.begin();
+ it != send_ssrcs_.end();
+ ++it) {
+ if (it->second == static_cast<VideoSendStream*>(send_stream)) {
+ send_stream_impl = it->second;
+ send_ssrcs_.erase(it);
+ break;
+ }
+ }
}
- // TODO(pbos): Remove it properly! Free the SSRCs!
- delete static_cast<VideoSendStream*>(send_stream);
+
+ assert(send_stream_impl != NULL);
+ delete send_stream_impl;
// TODO(pbos): Return its previous state
return NULL;
@@ -122,11 +136,25 @@
void VideoCall::DestroyReceiveStream(
webrtc::VideoReceiveStream* receive_stream) {
- if (receive_stream == NULL) {
- return;
+ assert(receive_stream != NULL);
+
+ VideoReceiveStream* receive_stream_impl = NULL;
+ {
+ WriteLockScoped write_lock(*receive_lock_);
+ for (std::map<uint32_t, VideoReceiveStream*>::iterator it =
+ receive_ssrcs_.begin();
+ it != receive_ssrcs_.end();
+ ++it) {
+ if (it->second == static_cast<VideoReceiveStream*>(receive_stream)) {
+ receive_stream_impl = it->second;
+ receive_ssrcs_.erase(it);
+ break;
+ }
+ }
}
- // TODO(pbos): Remove its SSRCs!
- delete static_cast<VideoReceiveStream*>(receive_stream);
+
+ assert(receive_stream_impl != NULL);
+ delete receive_stream_impl;
}
uint32_t VideoCall::SendBitrateEstimate() {
diff --git a/webrtc/video_engine/test/engine_tests.cc b/webrtc/video_engine/test/engine_tests.cc
index 61a0f5c..7a449af 100644
--- a/webrtc/video_engine/test/engine_tests.cc
+++ b/webrtc/video_engine/test/engine_tests.cc
@@ -256,13 +256,17 @@
void StopSending() {
frame_generator_capturer_->Stop();
- send_stream_->StopSend();
- receive_stream_->StopReceive();
+ if (send_stream_ != NULL)
+ send_stream_->StopSend();
+ if (receive_stream_ != NULL)
+ receive_stream_->StopReceive();
}
void DestroyStreams() {
- sender_call_->DestroySendStream(send_stream_);
- receiver_call_->DestroyReceiveStream(receive_stream_);
+ if (send_stream_ != NULL)
+ sender_call_->DestroySendStream(send_stream_);
+ if (receive_stream_ != NULL)
+ receiver_call_->DestroyReceiveStream(receive_stream_);
send_stream_= NULL;
receive_stream_ = NULL;
}
@@ -553,5 +557,59 @@
ReceivesPliAndRecovers(0);
}
+TEST_P(EngineTest, SurvivesIncomingRtpPacketsToDestroyedReceiveStream) {
+ class PacketInputObserver : public PacketReceiver {
+ public:
+ explicit PacketInputObserver(PacketReceiver* receiver)
+ : receiver_(receiver), delivered_packet_(EventWrapper::Create()) {}
+
+ EventTypeWrapper Wait() {
+ return delivered_packet_->Wait(30 * 1000);
+ }
+
+ private:
+ virtual bool DeliverPacket(const uint8_t* packet, size_t length) {
+ if (RtpHeaderParser::IsRtcp(packet, static_cast<int>(length))) {
+ return receiver_->DeliverPacket(packet, length);
+ } else {
+ EXPECT_FALSE(receiver_->DeliverPacket(packet, length));
+ delivered_packet_->Set();
+ return false;
+ }
+ }
+
+ PacketReceiver* receiver_;
+ scoped_ptr<EventWrapper> delivered_packet_;
+ };
+
+ test::DirectTransport send_transport, receive_transport;
+
+ CreateCalls(&send_transport, &receive_transport);
+ PacketInputObserver input_observer(receiver_call_->Receiver());
+
+ send_transport.SetReceiver(&input_observer);
+ receive_transport.SetReceiver(sender_call_->Receiver());
+
+ CreateTestConfigs();
+
+ CreateStreams();
+ CreateFrameGenerator();
+
+ StartSending();
+
+ receiver_call_->DestroyReceiveStream(receive_stream_);
+ receive_stream_ = NULL;
+
+ // Wait() waits for a received packet.
+ EXPECT_EQ(kEventSignaled, input_observer.Wait());
+
+ StopSending();
+
+ DestroyStreams();
+
+ send_transport.StopSending();
+ receive_transport.StopSending();
+}
+
INSTANTIATE_TEST_CASE_P(EngineTest, EngineTest, ::testing::Values(video_vga));
} // namespace webrtc
