Make MessageHandler cleanup optional.

As documented in webrtc:11908 this cleanup is fairly invasive and
when a part of a frequently executed code path, can be quite costly
in terms of performance overhead. This is currently the case with
synchronous calls between threads (Thread) as well with our proxy
api classes.

With this CL, all code in WebRTC should now either be using MessageHandlerAutoCleanup
or calling MessageHandler(false) explicitly.

Next steps will be to update external code to either depend on the
AutoCleanup variant, or call MessageHandler(false).

Changing the proxy classes to use TaskQueue set of concepts instead of
MessageHandler. This avoids the perf overhead related to the cleanup
above as well as incompatibility with the thread policy checks in
Thread that some current external users of the proxies would otherwise
run into (if we were to use Thread::Send() for synchronous call).

Following this we'll move the cleanup step into the AutoCleanup class
and an RTC_DCHECK that all calls to the MessageHandler are setting
the flag to false, before eventually removing the flag and make
MessageHandler pure virtual.

Bug: webrtc:11908
Change-Id: Idf4ff9bcc8438cb8c583777e282005e0bc511c8f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/183442
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32049}

api/proxy.cc

@@ -10,28 +10,3 @@
 
 #include "api/proxy.h"
 
-namespace webrtc {
-namespace internal {
-
-SynchronousMethodCall::SynchronousMethodCall(rtc::MessageHandler* proxy)
-    : proxy_(proxy) {}
-
-SynchronousMethodCall::~SynchronousMethodCall() = default;
-
-void SynchronousMethodCall::Invoke(const rtc::Location& posted_from,
-                                   rtc::Thread* t) {
-  if (t->IsCurrent()) {
-    proxy_->OnMessage(nullptr);
-  } else {
-    t->Post(posted_from, this, 0);
-    e_.Wait(rtc::Event::kForever);
-  }
-}
-
-void SynchronousMethodCall::OnMessage(rtc::Message*) {
-  proxy_->OnMessage(nullptr);
-  e_.Set();
-}
-
-}  // namespace internal
-}  // namespace webrtc

api/proxy.h

@@ -59,6 +59,8 @@
 #include <utility>
 
 #include "api/scoped_refptr.h"
+#include "api/task_queue/queued_task.h"
+#include "api/task_queue/task_queue_base.h"
 #include "rtc_base/event.h"
 #include "rtc_base/message_handler.h"
 #include "rtc_base/ref_counted_object.h"
@@ -96,27 +98,8 @@ class ReturnType<void> {
   void moved_result() {}
 };
 
-namespace internal {
-
-class RTC_EXPORT SynchronousMethodCall : public rtc::MessageData,
-                                         public rtc::MessageHandler {
- public:
-  explicit SynchronousMethodCall(rtc::MessageHandler* proxy);
-  ~SynchronousMethodCall() override;
-
-  void Invoke(const rtc::Location& posted_from, rtc::Thread* t);
-
- private:
-  void OnMessage(rtc::Message*) override;
-
-  rtc::Event e_;
-  rtc::MessageHandler* proxy_;
-};
-
-}  // namespace internal
-
 template <typename C, typename R, typename... Args>
-class MethodCall : public rtc::Message, public rtc::MessageHandler {
+class MethodCall : public QueuedTask {
  public:
   typedef R (C::*Method)(Args...);
   MethodCall(C* c, Method m, Args&&... args)
@@ -125,12 +108,21 @@ class MethodCall : public rtc::Message, public rtc::MessageHandler {
         args_(std::forward_as_tuple(std::forward<Args>(args)...)) {}
 
   R Marshal(const rtc::Location& posted_from, rtc::Thread* t) {
-    internal::SynchronousMethodCall(this).Invoke(posted_from, t);
+    if (t->IsCurrent()) {
+      Invoke(std::index_sequence_for<Args...>());
+    } else {
+      t->PostTask(std::unique_ptr<QueuedTask>(this));
+      event_.Wait(rtc::Event::kForever);
+    }
     return r_.moved_result();
   }
 
  private:
-  void OnMessage(rtc::Message*) { Invoke(std::index_sequence_for<Args...>()); }
+  bool Run() override {
+    Invoke(std::index_sequence_for<Args...>());
+    event_.Set();
+    return false;
+  }
 
   template <size_t... Is>
   void Invoke(std::index_sequence<Is...>) {
@@ -141,10 +133,11 @@ class MethodCall : public rtc::Message, public rtc::MessageHandler {
   Method m_;
   ReturnType<R> r_;
   std::tuple<Args&&...> args_;
+  rtc::Event event_;
 };
 
 template <typename C, typename R, typename... Args>
-class ConstMethodCall : public rtc::Message, public rtc::MessageHandler {
+class ConstMethodCall : public QueuedTask {
  public:
   typedef R (C::*Method)(Args...) const;
   ConstMethodCall(const C* c, Method m, Args&&... args)
@@ -153,12 +146,21 @@ class ConstMethodCall : public rtc::Message, public rtc::MessageHandler {
         args_(std::forward_as_tuple(std::forward<Args>(args)...)) {}
 
   R Marshal(const rtc::Location& posted_from, rtc::Thread* t) {
-    internal::SynchronousMethodCall(this).Invoke(posted_from, t);
+    if (t->IsCurrent()) {
+      Invoke(std::index_sequence_for<Args...>());
+    } else {
+      t->PostTask(std::unique_ptr<QueuedTask>(this));
+      event_.Wait(rtc::Event::kForever);
+    }
     return r_.moved_result();
   }
 
  private:
-  void OnMessage(rtc::Message*) { Invoke(std::index_sequence_for<Args...>()); }
+  bool Run() override {
+    Invoke(std::index_sequence_for<Args...>());
+    event_.Set();
+    return false;
+  }
 
   template <size_t... Is>
   void Invoke(std::index_sequence<Is...>) {
@@ -169,6 +171,7 @@ class ConstMethodCall : public rtc::Message, public rtc::MessageHandler {
   Method m_;
   ReturnType<R> r_;
   std::tuple<Args&&...> args_;
+  rtc::Event event_;
 };
 
 // Helper macros to reduce code duplication.

audio/null_audio_poller.cc

@@ -31,7 +31,8 @@ constexpr size_t kNumSamples = kSamplesPerSecond / 100;  // 10ms of samples
 }  // namespace
 
 NullAudioPoller::NullAudioPoller(AudioTransport* audio_transport)
-    : audio_transport_(audio_transport),
+    : MessageHandler(false),
+      audio_transport_(audio_transport),
       reschedule_at_(rtc::TimeMillis() + kPollDelayMs) {
   RTC_DCHECK(audio_transport);
   OnMessage(nullptr);  // Start the poll loop.

examples/peerconnection/client/peer_connection_client.cc

@@ -43,9 +43,15 @@ rtc::AsyncSocket* CreateClientSocket(int family) {
 }  // namespace
 
 PeerConnectionClient::PeerConnectionClient()
-    : callback_(NULL), resolver_(NULL), state_(NOT_CONNECTED), my_id_(-1) {}
+    : MessageHandler(false),
+      callback_(NULL),
+      resolver_(NULL),
+      state_(NOT_CONNECTED),
+      my_id_(-1) {}
 
-PeerConnectionClient::~PeerConnectionClient() {}
+PeerConnectionClient::~PeerConnectionClient() {
+  rtc::Thread::Current()->Clear(this);
+}
 
 void PeerConnectionClient::InitSocketSignals() {
   RTC_DCHECK(control_socket_.get() != NULL);

media/base/fake_network_interface.h

@@ -28,7 +28,7 @@ namespace cricket {
 
 // Fake NetworkInterface that sends/receives RTP/RTCP packets.
 class FakeNetworkInterface : public MediaChannel::NetworkInterface,
-                             public rtc::MessageHandler {
+                             public rtc::MessageHandlerAutoCleanup {
  public:
   FakeNetworkInterface()
       : thread_(rtc::Thread::Current()),

p2p/base/connection.h

@@ -71,7 +71,7 @@ class ConnectionRequest : public StunRequest {
 // Represents a communication link between a port on the local client and a
 // port on the remote client.
 class Connection : public CandidatePairInterface,
-                   public rtc::MessageHandler,
+                   public rtc::MessageHandlerAutoCleanup,
                    public sigslot::has_slots<> {
  public:
   struct SentPing {

p2p/base/p2p_transport_channel_unittest.cc

@@ -207,7 +207,7 @@ namespace cricket {
 // Note that this class is a base class for use by other tests, who will provide
 // specialized test behavior.
 class P2PTransportChannelTestBase : public ::testing::Test,
-                                    public rtc::MessageHandler,
+                                    public rtc::MessageHandlerAutoCleanup,
                                     public sigslot::has_slots<> {
  public:
   P2PTransportChannelTestBase()

p2p/base/port.h

@@ -160,7 +160,7 @@ typedef std::set<rtc::SocketAddress> ServerAddresses;
 // connections to similar mechanisms of the other client.  Subclasses of this
 // one add support for specific mechanisms like local UDP ports.
 class Port : public PortInterface,
-             public rtc::MessageHandler,
+             public rtc::MessageHandlerAutoCleanup,
              public sigslot::has_slots<> {
  public:
   // INIT: The state when a port is just created.

p2p/base/pseudo_tcp_unittest.cc

@@ -44,7 +44,7 @@ class PseudoTcpForTest : public cricket::PseudoTcp {
 };
 
 class PseudoTcpTestBase : public ::testing::Test,
-                          public rtc::MessageHandler,
+                          public rtc::MessageHandlerAutoCleanup,
                           public cricket::IPseudoTcpNotify {
  public:
   PseudoTcpTestBase()

p2p/base/stun_request.h

@@ -85,7 +85,7 @@ class StunRequestManager {
 
 // Represents an individual request to be sent.  The STUN message can either be
 // constructed beforehand or built on demand.
-class StunRequest : public rtc::MessageHandler {
+class StunRequest : public rtc::MessageHandlerAutoCleanup {
  public:
   StunRequest();
   explicit StunRequest(StunMessage* request);

p2p/base/turn_port_unittest.cc

@@ -148,7 +148,7 @@ class TestConnectionWrapper : public sigslot::has_slots<> {
 // (between local port and TURN server) of kSimulatedRtt.
 class TurnPortTest : public ::testing::Test,
                      public sigslot::has_slots<>,
-                     public rtc::MessageHandler {
+                     public rtc::MessageHandlerAutoCleanup {
  public:
   TurnPortTest()
       : ss_(new TurnPortTestVirtualSocketServer()),

p2p/base/turn_server.cc

@@ -59,7 +59,7 @@ enum {
 // Encapsulates a TURN permission.
 // The object is created when a create permission request is received by an
 // allocation, and self-deletes when its lifetime timer expires.
-class TurnServerAllocation::Permission : public rtc::MessageHandler {
+class TurnServerAllocation::Permission : public rtc::MessageHandlerAutoCleanup {
  public:
   Permission(rtc::Thread* thread, const rtc::IPAddress& peer);
   ~Permission() override;
@@ -79,7 +79,7 @@ class TurnServerAllocation::Permission : public rtc::MessageHandler {
 // Encapsulates a TURN channel binding.
 // The object is created when a channel bind request is received by an
 // allocation, and self-deletes when its lifetime timer expires.
-class TurnServerAllocation::Channel : public rtc::MessageHandler {
+class TurnServerAllocation::Channel : public rtc::MessageHandlerAutoCleanup {
  public:
   Channel(rtc::Thread* thread, int id, const rtc::SocketAddress& peer);
   ~Channel() override;

p2p/base/turn_server.h

@@ -66,7 +66,7 @@ class TurnServerConnection {
 // handles TURN messages (via HandleTurnMessage) and channel data messages
 // (via HandleChannelData) for this allocation when received by the server.
 // The object self-deletes and informs the server if its lifetime timer expires.
-class TurnServerAllocation : public rtc::MessageHandler,
+class TurnServerAllocation : public rtc::MessageHandlerAutoCleanup,
                              public sigslot::has_slots<> {
  public:
   TurnServerAllocation(TurnServer* server_,

p2p/client/basic_port_allocator.h

@@ -106,8 +106,9 @@ enum class SessionState {
               // process will be started.
 };
 
-class RTC_EXPORT BasicPortAllocatorSession : public PortAllocatorSession,
+class RTC_EXPORT BasicPortAllocatorSession
-                                             public rtc::MessageHandler {
+    : public PortAllocatorSession,
+      public rtc::MessageHandlerAutoCleanup {
  public:
   BasicPortAllocatorSession(BasicPortAllocator* allocator,
                             const std::string& content_name,
@@ -323,7 +324,7 @@ class TurnPort;
 
 // Performs the allocation of ports, in a sequenced (timed) manner, for a given
 // network and IP address.
-class AllocationSequence : public rtc::MessageHandler,
+class AllocationSequence : public rtc::MessageHandlerAutoCleanup,
                            public sigslot::has_slots<> {
  public:
   enum State {

pc/channel.h

@@ -71,7 +71,7 @@ struct CryptoParams;
 // NetworkInterface.
 
 class BaseChannel : public ChannelInterface,
-                    public rtc::MessageHandler,
+                    public rtc::MessageHandlerAutoCleanup,
                     public sigslot::has_slots<>,
                     public MediaChannel::NetworkInterface,
                     public webrtc::RtpPacketSinkInterface {

pc/peer_connection.cc

@@ -1027,7 +1027,8 @@ void ExtractSharedMediaSessionOptions(
 PeerConnection::PeerConnection(PeerConnectionFactory* factory,
                                std::unique_ptr<RtcEventLog> event_log,
                                std::unique_ptr<Call> call)
-    : factory_(factory),
+    : MessageHandler(false),
+      factory_(factory),
       event_log_(std::move(event_log)),
       event_log_ptr_(event_log_.get()),
       operations_chain_(rtc::OperationsChain::Create()),

pc/remote_audio_source.cc

@@ -50,7 +50,8 @@ class RemoteAudioSource::AudioDataProxy : public AudioSinkInterface {
 };
 
 RemoteAudioSource::RemoteAudioSource(rtc::Thread* worker_thread)
-    : main_thread_(rtc::Thread::Current()),
+    : MessageHandler(false),
+      main_thread_(rtc::Thread::Current()),
       worker_thread_(worker_thread),
       state_(MediaSourceInterface::kLive) {
   RTC_DCHECK(main_thread_);

pc/test/fake_audio_capture_module.h

@@ -33,7 +33,7 @@ class Thread;
 }  // namespace rtc
 
 class FakeAudioCaptureModule : public webrtc::AudioDeviceModule,
-                               public rtc::MessageHandler {
+                               public rtc::MessageHandlerAutoCleanup {
  public:
   typedef uint16_t Sample;
 

pc/test/fake_rtc_certificate_generator.h

@@ -118,7 +118,7 @@ static const rtc::RTCCertificatePEM kEcdsaPems[] = {
 
 class FakeRTCCertificateGenerator
     : public rtc::RTCCertificateGeneratorInterface,
-      public rtc::MessageHandler {
+      public rtc::MessageHandlerAutoCleanup {
  public:
   typedef rtc::TypedMessageData<
       rtc::scoped_refptr<rtc::RTCCertificateGeneratorCallback> >

pc/webrtc_session_description_factory.cc

@@ -130,7 +130,8 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory(
     std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
     const rtc::scoped_refptr<rtc::RTCCertificate>& certificate,
     UniqueRandomIdGenerator* ssrc_generator)
-    : signaling_thread_(signaling_thread),
+    : MessageHandler(false),
+      signaling_thread_(signaling_thread),
       session_desc_factory_(channel_manager,
                             &transport_desc_factory_,
                             ssrc_generator),

rtc_base/async_invoker.h

@@ -87,7 +87,7 @@ namespace rtc {
 //   destruction. This can be done by starting each chain of invocations on the
 //   same thread on which it will be destroyed, or by using some other
 //   synchronization method.
-class AsyncInvoker : public MessageHandler {
+class AsyncInvoker : public MessageHandlerAutoCleanup {
  public:
   AsyncInvoker();
   ~AsyncInvoker() override;

rtc_base/deprecated/recursive_critical_section_unittest.cc

@@ -78,7 +78,7 @@ class CompareAndSwapVerifier {
   int zero_count_;
 };
 
-class RunnerBase : public MessageHandler {
+class RunnerBase : public MessageHandlerAutoCleanup {
  public:
   explicit RunnerBase(int value)
       : threads_active_(0),

rtc_base/deprecated/signal_thread.h

@@ -45,7 +45,7 @@ namespace rtc {
 ///////////////////////////////////////////////////////////////////////////////
 
 class DEPRECATED_SignalThread : public sigslot::has_slots<>,
-                                protected MessageHandler {
+                                protected MessageHandlerAutoCleanup {
  public:
   DEPRECATED_SignalThread();
 

rtc_base/fake_network.h

@@ -31,7 +31,8 @@ const int kFakeIPv4NetworkPrefixLength = 24;
 const int kFakeIPv6NetworkPrefixLength = 64;
 
 // Fake network manager that allows us to manually specify the IPs to use.
-class FakeNetworkManager : public NetworkManagerBase, public MessageHandler {
+class FakeNetworkManager : public NetworkManagerBase,
+                           public MessageHandlerAutoCleanup {
  public:
   FakeNetworkManager() {}
 

rtc_base/message_handler.cc

@@ -15,7 +15,18 @@
 namespace rtc {
 
 MessageHandler::~MessageHandler() {
+  if (auto_cleanup_) {
+    // Note that even though this clears currently pending messages for the
+    // message handler, it's still racy since it doesn't prevent threads that
+    // might be in the process of posting new messages with would-be dangling
+    // pointers.
+    // This is related to the design of Message having a raw pointer.
+    // We could consider whether it would be safer to require message handlers
+    // to be reference counted (as some are).
     ThreadManager::Clear(this);
   }
+}
+
+MessageHandlerAutoCleanup::~MessageHandlerAutoCleanup() {}
 
 }  // namespace rtc

rtc_base/message_handler.h

@@ -21,17 +21,41 @@ namespace rtc {
 
 struct Message;
 
-// Messages get dispatched to a MessageHandler
+// MessageQueue/Thread Messages get dispatched to a MessageHandler via the
+// |OnMessage()| callback method.
+//
+// Note: Besides being an interface, the class can perform automatic cleanup
+// in the destructor.
+// TODO(bugs.webrtc.org/11908): The |auto_cleanup| parameter and associated
+// logic is a temporary step while changing the MessageHandler class to be
+// a pure virtual interface. The automatic cleanup step involves a number of
+// complex operations and as part of this interface, can easily go by unnoticed
+// and bundled into situations where it's not needed.
 class RTC_EXPORT MessageHandler {
  public:
   virtual ~MessageHandler();
   virtual void OnMessage(Message* msg) = 0;
 
  protected:
-  MessageHandler() {}
+  // TODO(bugs.webrtc.org/11908): The |auto_cleanup| parameter needs to have a
+  // backwards compatible default value while external code is being updated.
+  explicit MessageHandler(bool auto_cleanup = true)
+      : auto_cleanup_(auto_cleanup) {}
 
  private:
   RTC_DISALLOW_COPY_AND_ASSIGN(MessageHandler);
+  const bool auto_cleanup_;
+};
+
+class RTC_EXPORT MessageHandlerAutoCleanup : public MessageHandler {
+ public:
+  ~MessageHandlerAutoCleanup() override;
+
+ protected:
+  MessageHandlerAutoCleanup() : MessageHandler(true) {}
+
+ private:
+  RTC_DISALLOW_COPY_AND_ASSIGN(MessageHandlerAutoCleanup);
 };
 
 }  // namespace rtc

rtc_base/network.h

@@ -224,7 +224,7 @@ class RTC_EXPORT NetworkManagerBase : public NetworkManager {
 // Basic implementation of the NetworkManager interface that gets list
 // of networks using OS APIs.
 class RTC_EXPORT BasicNetworkManager : public NetworkManagerBase,
-                                       public MessageHandler,
+                                       public MessageHandlerAutoCleanup,
                                        public sigslot::has_slots<> {
  public:
   BasicNetworkManager();

rtc_base/network_monitor.cc

@@ -25,8 +25,11 @@ NetworkMonitorInterface::NetworkMonitorInterface() {}
 
 NetworkMonitorInterface::~NetworkMonitorInterface() {}
 
-NetworkMonitorBase::NetworkMonitorBase() : worker_thread_(Thread::Current()) {}
+NetworkMonitorBase::NetworkMonitorBase()
-NetworkMonitorBase::~NetworkMonitorBase() {}
+    : MessageHandler(false), worker_thread_(Thread::Current()) {}
+NetworkMonitorBase::~NetworkMonitorBase() {
+  worker_thread_->Clear(this);
+}
 
 void NetworkMonitorBase::OnNetworksChanged() {
   RTC_LOG(LS_VERBOSE) << "Network change is received at the network monitor";

rtc_base/null_socket_server_unittest.cc

@@ -25,7 +25,8 @@ namespace rtc {
 
 static const uint32_t kTimeout = 5000U;
 
-class NullSocketServerTest : public ::testing::Test, public MessageHandler {
+class NullSocketServerTest : public ::testing::Test,
+                             public MessageHandlerAutoCleanup {
  protected:
   void OnMessage(Message* message) override { ss_.WakeUp(); }
 

rtc_base/openssl_adapter.h

@@ -32,7 +32,8 @@
 
 namespace rtc {
 
-class OpenSSLAdapter final : public SSLAdapter, public MessageHandler {
+class OpenSSLAdapter final : public SSLAdapter,
+                             public MessageHandlerAutoCleanup {
  public:
   static bool InitializeSSL();
   static bool CleanupSSL();

rtc_base/rtc_certificate_generator.cc

@@ -40,7 +40,7 @@ enum {
 // request. We are using a separate helper class so that a generation request
 // can outlive the |RTCCertificateGenerator| that spawned it.
 class RTCCertificateGenerationTask : public RefCountInterface,
-                                     public MessageHandler {
+                                     public MessageHandlerAutoCleanup {
  public:
   RTCCertificateGenerationTask(
       Thread* signaling_thread,

rtc_base/socket_unittest.cc

@@ -696,7 +696,7 @@ void SocketTest::DeleteInReadCallbackInternal(const IPAddress& loopback) {
   EXPECT_TRUE_WAIT(deleter.deleted(), kTimeout);
 }
 
-class Sleeper : public MessageHandler {
+class Sleeper : public MessageHandlerAutoCleanup {
  public:
   void OnMessage(Message* msg) override { Thread::Current()->SleepMs(500); }
 };

rtc_base/stream.h

@@ -53,7 +53,7 @@ struct StreamEventData : public MessageData {
   StreamEventData(int ev, int er) : events(ev), error(er) {}
 };
 
-class RTC_EXPORT StreamInterface : public MessageHandler {
+class RTC_EXPORT StreamInterface : public MessageHandlerAutoCleanup {
  public:
   enum { MSG_POST_EVENT = 0xF1F1, MSG_MAX = MSG_POST_EVENT };
 

rtc_base/synchronization/mutex_unittest.cc

@@ -77,7 +77,7 @@ class MutexLockLocker {
 };
 
 template <class MutexType, class MutexLocker>
-class LockRunner : public MessageHandler {
+class LockRunner : public rtc::MessageHandlerAutoCleanup {
  public:
   template <typename... Args>
   explicit LockRunner(Args... args)

rtc_base/thread.cc

@@ -74,7 +74,7 @@ const int kSlowDispatchLoggingThreshold = 50;  // 50 ms
 
 class MessageHandlerWithTask final : public MessageHandler {
  public:
-  MessageHandlerWithTask() = default;
+  MessageHandlerWithTask() : MessageHandler(false) {}
 
   void OnMessage(Message* msg) override {
     static_cast<rtc_thread_internal::MessageLikeTask*>(msg->pdata)->Run();
@@ -961,7 +961,7 @@ void Thread::InvokeInternal(const Location& posted_from,
   class FunctorMessageHandler : public MessageHandler {
    public:
     explicit FunctorMessageHandler(rtc::FunctionView<void()> functor)
-        : functor_(functor) {}
+        : MessageHandler(false), functor_(functor) {}
     void OnMessage(Message* msg) override { functor_(); }
 
    private:

rtc_base/thread.h

@@ -339,6 +339,7 @@ class RTC_LOCKABLE RTC_EXPORT Thread : public webrtc::TaskQueueBase {
   // will be used only for reference-based comparison, so instance can be safely
   // deleted. If NDEBUG is defined and DCHECK_ALWAYS_ON is undefined do nothing.
   void AllowInvokesToThread(Thread* thread);
+
   // If NDEBUG is defined and DCHECK_ALWAYS_ON is undefined do nothing.
   void DisallowAllInvokes();
   // Returns true if |target| was allowed by AllowInvokesToThread() or if no
@@ -533,6 +534,7 @@ class RTC_LOCKABLE RTC_EXPORT Thread : public webrtc::TaskQueueBase {
  private:
   class QueuedTaskHandler final : public MessageHandler {
    public:
+    QueuedTaskHandler() : MessageHandler(false) {}
     void OnMessage(Message* msg) override;
   };
 

rtc_base/thread_unittest.cc

@@ -96,7 +96,7 @@ class SocketClient : public TestGenerator, public sigslot::has_slots<> {
 };
 
 // Receives messages and sends on a socket.
-class MessageClient : public MessageHandler, public TestGenerator {
+class MessageClient : public MessageHandlerAutoCleanup, public TestGenerator {
  public:
   MessageClient(Thread* pth, Socket* socket) : socket_(socket) {}
 
@@ -574,7 +574,7 @@ TEST_F(ThreadQueueTest, DisposeNotLocked) {
   EXPECT_FALSE(was_locked);
 }
 
-class DeletedMessageHandler : public MessageHandler {
+class DeletedMessageHandler : public MessageHandlerAutoCleanup {
  public:
   explicit DeletedMessageHandler(bool* deleted) : deleted_(deleted) {}
   ~DeletedMessageHandler() override { *deleted_ = true; }
@@ -664,12 +664,13 @@ TEST(ThreadManager, ProcessAllMessageQueuesWithClearedQueue) {
   ThreadManager::ProcessAllMessageQueuesForTesting();
 }
 
-class RefCountedHandler : public MessageHandler, public rtc::RefCountInterface {
+class RefCountedHandler : public MessageHandlerAutoCleanup,
+                          public rtc::RefCountInterface {
  public:
   void OnMessage(Message* msg) override {}
 };
 
-class EmptyHandler : public MessageHandler {
+class EmptyHandler : public MessageHandlerAutoCleanup {
  public:
   void OnMessage(Message* msg) override {}
 };

rtc_base/virtual_socket_server.h

@@ -305,7 +305,7 @@ class VirtualSocketServer : public SocketServer, public sigslot::has_slots<> {
 // Implements the socket interface using the virtual network.  Packets are
 // passed as messages using the message queue of the socket server.
 class VirtualSocket : public AsyncSocket,
-                      public MessageHandler,
+                      public MessageHandlerAutoCleanup,
                       public sigslot::has_slots<> {
  public:
   VirtualSocket(VirtualSocketServer* server, int family, int type, bool async);

rtc_base/virtual_socket_unittest.cc

@@ -53,7 +53,7 @@ using webrtc::testing::SSE_WRITE;
 using webrtc::testing::StreamSink;
 
 // Sends at a constant rate but with random packet sizes.
-struct Sender : public MessageHandler {
+struct Sender : public MessageHandlerAutoCleanup {
   Sender(Thread* th, AsyncSocket* s, uint32_t rt)
       : thread(th),
         socket(std::make_unique<AsyncUDPSocket>(s)),
@@ -99,7 +99,8 @@ struct Sender : public MessageHandler {
   char dummy[4096];
 };
 
-struct Receiver : public MessageHandler, public sigslot::has_slots<> {
+struct Receiver : public MessageHandlerAutoCleanup,
+                  public sigslot::has_slots<> {
   Receiver(Thread* th, AsyncSocket* s, uint32_t bw)
       : thread(th),
         socket(std::make_unique<AsyncUDPSocket>(s)),

sdk/android/src/jni/pc/peer_connection_factory.cc

@@ -82,7 +82,9 @@ void PostJavaCallback(JNIEnv* env,
     JavaAsyncCallback(JNIEnv* env,
                       const JavaRef<jobject>& j_object,
                       JavaMethodPointer java_method_pointer)
-        : j_object_(env, j_object), java_method_pointer_(java_method_pointer) {}
+        : rtc::MessageHandler(false),
+          j_object_(env, j_object),
+          java_method_pointer_(java_method_pointer) {}
 
     void OnMessage(rtc::Message*) override {
       java_method_pointer_(AttachCurrentThreadIfNeeded(), j_object_);

sdk/objc/native/src/audio/audio_device_ios.mm

@@ -101,7 +101,8 @@ static void LogDeviceInfo() {
 #endif  // !defined(NDEBUG)
 
 AudioDeviceIOS::AudioDeviceIOS()
-    : audio_device_buffer_(nullptr),
+    : MessageHandler(false),
+      audio_device_buffer_(nullptr),
       audio_unit_(nullptr),
       recording_(0),
       playing_(0),
@@ -124,6 +125,7 @@ AudioDeviceIOS::AudioDeviceIOS()
 AudioDeviceIOS::~AudioDeviceIOS() {
   RTC_DCHECK(thread_checker_.IsCurrent());
   LOGI() << "~dtor" << ios::GetCurrentThreadDescription();
+  thread_->Clear(this);
   Terminate();
   audio_session_observer_ = nil;
 }