From 4f7486ab3bf9a13d0c8a19e80abb8aa2ee1d666a Mon Sep 17 00:00:00 2001
From: Harald Alvestrand <hta@webrtc.org>
Date: Thu, 2 Jun 2022 11:35:49 +0000
Subject: [PATCH] Destroy peerconnections in test when they refer to on-stack
 mocks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adds a function to PeerConnectionIntegrationBaseTest to stop and destroy
the caller and callee objects. This should take care of dangling pointers.

Before this change, the affected test would crash randomly - typically
detected within a few minutes of a gtest-repeat=-1 run.

After this change, it has not crashed in 15 minutes of running.

Bug: webrtc:12592
Change-Id: I9980f8974015bf2b2104fcb83c2ca0d677d03c3e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264555
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37096}
---
 pc/peer_connection_integrationtest.cc |  3 ++-
 pc/test/integration_test_helpers.h    | 14 ++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
index 940d7fdc74..8afd807ff7 100644
--- a/pc/peer_connection_integrationtest.cc
+++ b/pc/peer_connection_integrationtest.cc
@@ -1797,7 +1797,7 @@ constexpr int kOnlyLocalPorts = cricket::PORTALLOCATOR_DISABLE_STUN |
 // Use a mock resolver to resolve the hostname back to the original IP on both
 // sides and check that the ICE connection connects.
 // TODO(bugs.webrtc.org/12590): Flaky on Windows and on Linux MSAN.
-#if defined(WEBRTC_WIN) || defined(WEBRTC_LINUX)
+#if defined(WEBRTC_WIN)
 #define MAYBE_IceStatesReachCompletionWithRemoteHostname \
   DISABLED_IceStatesReachCompletionWithRemoteHostname
 #else
@@ -1856,6 +1856,7 @@ TEST_P(PeerConnectionIntegrationTest,
   EXPECT_METRIC_EQ(1, webrtc::metrics::NumEvents(
                           "WebRTC.PeerConnection.CandidatePairType_UDP",
                           webrtc::kIceCandidatePairHostNameHostName));
+  DestroyPeerConnections();
 }
 
 #endif  // !defined(THREAD_SANITIZER)
diff --git a/pc/test/integration_test_helpers.h b/pc/test/integration_test_helpers.h
index a1d3a548e5..e733bea5c7 100644
--- a/pc/test/integration_test_helpers.h
+++ b/pc/test/integration_test_helpers.h
@@ -1715,6 +1715,20 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test {
 
   PeerConnectionIntegrationWrapper* caller() { return caller_.get(); }
 
+  // Destroy peerconnections.
+  // This can be used to ensure that all pointers to on-stack mocks
+  // get dropped before exit.
+  void DestroyPeerConnections() {
+    if (caller_) {
+      caller_->pc()->Close();
+    }
+    if (callee_) {
+      callee_->pc()->Close();
+    }
+    caller_.reset();
+    callee_.reset();
+  }
+
   // Set the `caller_` to the `wrapper` passed in and return the
   // original `caller_`.
   PeerConnectionIntegrationWrapper* SetCallerPcWrapperAndReturnCurrent(