From 9f1104731e9df2d9a56f78f8d2b32f92b3470935 Mon Sep 17 00:00:00 2001
From: Artem Titov <titovartem@webrtc.org>
Date: Thu, 7 Jul 2022 17:00:26 +0200
Subject: [PATCH] [PCLF] Fix deadlock when stats are requested during peer
 destruction

Bug: b/238308795
Change-Id: If420846a73df22ed07184d1803bf35295a88ecff
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268148
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37491}
---
 test/pc/e2e/BUILD.gn     |  1 +
 test/pc/e2e/test_peer.cc | 11 ++++++++++-
 test/pc/e2e/test_peer.h  | 10 +++++++---
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/test/pc/e2e/BUILD.gn b/test/pc/e2e/BUILD.gn
index 4dc45efeb7..4f8c3e429f 100644
--- a/test/pc/e2e/BUILD.gn
+++ b/test/pc/e2e/BUILD.gn
@@ -236,6 +236,7 @@ if (!build_with_chromium) {
         "../../../api:peer_connection_quality_test_fixture_api",
         "../../../api:scoped_refptr",
         "../../../api:sequence_checker",
+        "../../../api/task_queue:pending_task_safety_flag",
         "../../../modules/audio_processing:api",
         "../../../pc:peerconnection_wrapper",
         "../../../rtc_base:logging",
diff --git a/test/pc/e2e/test_peer.cc b/test/pc/e2e/test_peer.cc
index 4f801f5c12..d978f10665 100644
--- a/test/pc/e2e/test_peer.cc
+++ b/test/pc/e2e/test_peer.cc
@@ -75,6 +75,12 @@ void TestPeer::SetVideoSubscription(VideoSubscription subscription) {
   configurable_params_.video_subscription = std::move(subscription);
 }
 
+void TestPeer::GetStats(RTCStatsCollectorCallback* callback) {
+  pc()->signaling_thread()->PostTask(
+      SafeTask(signaling_thread_task_safety_,
+               [this, callback]() { pc()->GetStats(callback); }));
+}
+
 bool TestPeer::SetRemoteDescription(
     std::unique_ptr<SessionDescriptionInterface> desc,
     std::string* error_out) {
@@ -115,6 +121,7 @@ bool TestPeer::AddIceCandidates(
 }
 
 void TestPeer::Close() {
+  signaling_thread_task_safety_->SetNotAlive();
   wrapper_->pc()->Close();
   remote_ice_candidates_.clear();
   audio_processing_ = nullptr;
@@ -139,7 +146,9 @@ TestPeer::TestPeer(
                                                        std::move(pc),
                                                        std::move(observer))),
       video_sources_(std::move(video_sources)),
-      audio_processing_(audio_processing) {}
+      audio_processing_(audio_processing) {
+  signaling_thread_task_safety_ = PendingTaskSafetyFlag::CreateDetached();
+}
 
 }  // namespace webrtc_pc_e2e
 }  // namespace webrtc
diff --git a/test/pc/e2e/test_peer.h b/test/pc/e2e/test_peer.h
index 72399f50cc..741e2e73f1 100644
--- a/test/pc/e2e/test_peer.h
+++ b/test/pc/e2e/test_peer.h
@@ -20,6 +20,7 @@
 #include "api/scoped_refptr.h"
 #include "api/sequence_checker.h"
 #include "api/set_remote_description_observer_interface.h"
+#include "api/task_queue/pending_task_safety_flag.h"
 #include "api/test/frame_generator_interface.h"
 #include "api/test/peerconnection_quality_test_fixture.h"
 #include "pc/peer_connection_wrapper.h"
@@ -47,9 +48,7 @@ class TestPeer final : public StatsProvider {
   void SetVideoSubscription(
       PeerConnectionE2EQualityTestFixture::VideoSubscription subscription);
 
-  void GetStats(RTCStatsCollectorCallback* callback) override {
-    pc()->GetStats(callback);
-  }
+  void GetStats(RTCStatsCollectorCallback* callback) override;
 
   PeerConfigurerImpl::VideoSource ReleaseVideoSource(size_t i) {
     RTC_CHECK(wrapper_) << "TestPeer is already closed";
@@ -168,6 +167,11 @@ class TestPeer final : public StatsProvider {
   mutable Mutex mutex_;
   ConfigurableParams configurable_params_ RTC_GUARDED_BY(mutex_);
 
+  // Safety flag to protect all tasks posted on the signaling thread to not be
+  // executed after `wrapper_` object is destructed.
+  rtc::scoped_refptr<PendingTaskSafetyFlag> signaling_thread_task_safety_ =
+      nullptr;
+
   // Keeps ownership of worker thread. It has to be destroyed after `wrapper_`.
   std::unique_ptr<rtc::Thread> worker_thread_;
   std::unique_ptr<PeerConnectionWrapper> wrapper_;