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
This commit is contained in:
parent
164c4f71ba
commit
95e51f509c
@ -95,11 +95,25 @@ VideoSendStream* VideoCall::CreateSendStream(
|
||||
|
||||
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 @@ VideoReceiveStream* VideoCall::CreateReceiveStream(
|
||||
|
||||
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() {
|
||||
|
@ -256,13 +256,17 @@ class EngineTest : public ::testing::TestWithParam<EngineTestParams> {
|
||||
|
||||
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 @@ TEST_P(EngineTest, DISABLED_ReceivesPliAndRecoversWithoutNack) {
|
||||
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
|
||||
|
Loading…
x
Reference in New Issue
Block a user