From 83829687815e5d72234e24bf23b31140a8e830be Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Fri, 25 Feb 2022 11:03:15 +0000 Subject: [PATCH] Reland "Remove workaround in AutoSocketServerThread that isn't needed anymore." This reverts commit e4d3952bf0b05b825992f1023a106f3bcf34946e. Reason for revert: Speculative revert. Original change's description: > Revert "Remove workaround in AutoSocketServerThread that isn't needed anymore." > > This reverts commit 44156fa024cbf12f052a35571ac91bc9907be6c3. > > Reason for revert: Needed in order to revert https://webrtc-review.googlesource.com/c/src/+/249941, which introduced a crash > > Original change's description: > > Remove workaround in AutoSocketServerThread that isn't needed anymore. > > > > Cleanup steps for the Connection class have changed as of: > > https://webrtc-review.googlesource.com/c/src/+/249941 > > > > However, it turns out that the PortTest suite still needs it, so the > > workaround has migrated to there. > > > > Bug: none > > Change-Id: Ia68f47b6c65b3a8fd5e8c04d70a43d15ba1a6422 > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/250223 > > Reviewed-by: Niels Moller > > Commit-Queue: Tomas Gunnarsson > > Cr-Commit-Position: refs/heads/main@{#35894} > > Bug: none > Change-Id: I13a4a79ebcb864054d14c1ba7726e18e044e3bd4 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/252542 > Auto-Submit: Taylor Brandstetter > Reviewed-by: Mirko Bonadei > Commit-Queue: Mirko Bonadei > Reviewed-by: Tomas Gunnarsson > Commit-Queue: Tomas Gunnarsson > Cr-Commit-Position: refs/heads/main@{#36076} No-Try: True Bug: none Change-Id: If39bb2f26349c42c2377ed9f80c26eb18d90869f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/252585 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Reviewed-by: Tomas Gunnarsson Commit-Queue: Mirko Bonadei Cr-Commit-Position: refs/heads/main@{#36082} --- p2p/base/port_unittest.cc | 7 +++++++ rtc_base/thread.cc | 5 ----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc index b27a527477..6ab40a66ad 100644 --- a/p2p/base/port_unittest.cc +++ b/p2p/base/port_unittest.cc @@ -415,6 +415,13 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { role_conflict_(false), ports_destroyed_(0) {} + ~PortTest() { + // Workaround for tests that trigger async destruction of objects that we + // need to give an opportunity here to run, before proceeding with other + // teardown. + rtc::Thread::Current()->ProcessMessages(0); + } + protected: std::string password() { return password_; } diff --git a/rtc_base/thread.cc b/rtc_base/thread.cc index aaf6c19b82..307d499255 100644 --- a/rtc_base/thread.cc +++ b/rtc_base/thread.cc @@ -1223,11 +1223,6 @@ AutoSocketServerThread::AutoSocketServerThread(SocketServer* ss) AutoSocketServerThread::~AutoSocketServerThread() { RTC_DCHECK(ThreadManager::Instance()->CurrentThread() == this); - // Some tests post destroy messages to this thread. To avoid memory - // leaks, we have to process those messages. In particular - // P2PTransportChannelPingTest, relying on the message posted in - // cricket::Connection::Destroy. - ProcessMessages(0); // Stop and destroy the thread before clearing it as the current thread. // Sometimes there are messages left in the Thread that will be // destroyed by DoDestroy, and sometimes the destructors of the message and/or