commit 792df6b4b98d9d5f6513010a77f36c54db266cb7
author Danil Chapovalov <danilchap@webrtc.org> Fri Sep 07 13:03:32 2018 +0200
committer Commit Bot <commit-bot@chromium.org> Fri Sep 07 15:34:08 2018 +0000
tree d9341c4ce645db5faf6b75c73a3881fd82771e53
parent d7027dc081cb96791b51524210aa249eb301c056

Make RtcpTransceiver destructor non-blocking

At cost of removing assumption callbacks can't be used after destructor.

Bug: webrtc:8239
Change-Id: Id79f7553528cf6c102d3ee0bf7aa2de5b0437d2a
Reviewed-on: https://webrtc-review.googlesource.com/98860
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24632}

modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc

diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc
index 1ef1998..2ea5bc9 100644
--- a/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc
@@ -47,8 +47,8 @@
 }
 
 TEST(RtcpTransceiverTest, SendsRtcpOnTaskQueueWhenCreatedOffTaskQueue) {
-  rtc::TaskQueue queue("rtcp");
   MockTransport outgoing_transport;
+  rtc::TaskQueue queue("rtcp");
   RtcpTransceiverConfig config;
   config.outgoing_transport = &outgoing_transport;
   config.task_queue = &queue;
@@ -64,8 +64,8 @@
 }
 
 TEST(RtcpTransceiverTest, SendsRtcpOnTaskQueueWhenCreatedOnTaskQueue) {
-  rtc::TaskQueue queue("rtcp");
   MockTransport outgoing_transport;
+  rtc::TaskQueue queue("rtcp");
   RtcpTransceiverConfig config;
   config.outgoing_transport = &outgoing_transport;
   config.task_queue = &queue;
@@ -83,39 +83,62 @@
   WaitPostedTasks(&queue);
 }
 
-TEST(RtcpTransceiverTest, CanBeDestroyedOnTaskQueueAfterStop) {
-  rtc::TaskQueue queue("rtcp");
+TEST(RtcpTransceiverTest, CanBeDestroyedOnTaskQueue) {
   NiceMock<MockTransport> outgoing_transport;
+  rtc::TaskQueue queue("rtcp");
   RtcpTransceiverConfig config;
   config.outgoing_transport = &outgoing_transport;
   config.task_queue = &queue;
   auto rtcp_transceiver = absl::make_unique<RtcpTransceiver>(config);
-  rtcp_transceiver->Stop(rtc::NewClosure([] {}));
 
-  queue.PostTask([&] { rtcp_transceiver.reset(); });
+  queue.PostTask([&] {
+    // Insert a packet just before destruction to test for races.
+    rtcp_transceiver->SendCompoundPacket();
+    rtcp_transceiver.reset();
+  });
   WaitPostedTasks(&queue);
 }
 
-TEST(RtcpTransceiverTest, CanBeDestroyedWithoutBlockingAfterStop) {
+TEST(RtcpTransceiverTest, CanBeDestroyedWithoutBlocking) {
   rtc::TaskQueue queue("rtcp");
   NiceMock<MockTransport> outgoing_transport;
   RtcpTransceiverConfig config;
   config.outgoing_transport = &outgoing_transport;
   config.task_queue = &queue;
-  auto rtcp_transceiver = absl::make_unique<RtcpTransceiver>(config);
+  auto* rtcp_transceiver = new RtcpTransceiver(config);
   rtcp_transceiver->SendCompoundPacket();
 
-  rtc::Event heavy_task(false, false);
-  queue.PostTask(
-      rtc::NewClosure([&] { EXPECT_TRUE(heavy_task.Wait(kTimeoutMs)); }));
   rtc::Event done(false, false);
-  rtcp_transceiver->Stop(rtc::NewClosure([&done] { done.Set(); }));
-  rtcp_transceiver = nullptr;
+  rtc::Event heavy_task(false, false);
+  queue.PostTask([&] {
+    EXPECT_TRUE(heavy_task.Wait(kTimeoutMs));
+    done.Set();
+  });
+  delete rtcp_transceiver;
 
   heavy_task.Set();
   EXPECT_TRUE(done.Wait(kTimeoutMs));
 }
 
+TEST(RtcpTransceiverTest, MaySendPacketsAfterDestructor) {  // i.e. Be careful!
+  NiceMock<MockTransport> outgoing_transport;  // Must outlive queue below.
+  rtc::TaskQueue queue("rtcp");
+  RtcpTransceiverConfig config;
+  config.outgoing_transport = &outgoing_transport;
+  config.task_queue = &queue;
+  auto* rtcp_transceiver = new RtcpTransceiver(config);
+
+  rtc::Event heavy_task(false, false);
+  queue.PostTask([&] { EXPECT_TRUE(heavy_task.Wait(kTimeoutMs)); });
+  rtcp_transceiver->SendCompoundPacket();
+  delete rtcp_transceiver;
+
+  EXPECT_CALL(outgoing_transport, SendRtcp);
+  heavy_task.Set();
+
+  WaitPostedTasks(&queue);
+}
+
 // Use rtp timestamp to distinguish different incoming sender reports.
 rtc::CopyOnWriteBuffer CreateSenderReport(uint32_t ssrc, uint32_t rtp_time) {
   webrtc::rtcp::SenderReport sr;