Fix memory leak in Thread::PostTask.
Use MessageData rather than MessageHandler to refer
to allocated storage.
That way, MessageQueue will delete storage for us if the
thread object is stopped before the Message is handled.
Leak seems triggered by the
RTCStatsIntegrationTest.GetsStatsWhileClosingPeerConnection
test.
Bug: webrtc:9714
Change-Id: I9e1255a3b6f16a763568744775ec0b3aef671227
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/136684
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27971}
diff --git a/rtc_base/thread.h b/rtc_base/thread.h
index 80720e0..b234c0b 100644
--- a/rtc_base/thread.h
+++ b/rtc_base/thread.h
@@ -38,23 +38,40 @@
namespace rtc_thread_internal {
-template <class FunctorT>
-class SingleMessageHandlerWithFunctor final : public MessageHandler {
+class MessageLikeTask : public MessageData {
public:
- explicit SingleMessageHandlerWithFunctor(FunctorT&& functor)
+ virtual void Run() = 0;
+};
+
+template <class FunctorT>
+class MessageWithFunctor final : public MessageLikeTask {
+ public:
+ explicit MessageWithFunctor(FunctorT&& functor)
: functor_(std::forward<FunctorT>(functor)) {}
- void OnMessage(Message* msg) override {
- functor_();
- delete this;
- }
+ void Run() override { functor_(); }
private:
- ~SingleMessageHandlerWithFunctor() override {}
+ ~MessageWithFunctor() override {}
typename std::remove_reference<FunctorT>::type functor_;
- RTC_DISALLOW_COPY_AND_ASSIGN(SingleMessageHandlerWithFunctor);
+ RTC_DISALLOW_COPY_AND_ASSIGN(MessageWithFunctor);
+};
+
+class MessageHandlerWithTask final : public MessageHandler {
+ public:
+ MessageHandlerWithTask() = default;
+
+ void OnMessage(Message* msg) override {
+ static_cast<MessageLikeTask*>(msg->pdata)->Run();
+ delete msg->pdata;
+ }
+
+ private:
+ ~MessageHandlerWithTask() override {}
+
+ RTC_DISALLOW_COPY_AND_ASSIGN(MessageHandlerWithTask);
};
} // namespace rtc_thread_internal
@@ -221,7 +238,8 @@
// Posts a task to invoke the functor on |this| thread asynchronously, i.e.
// without blocking the thread that invoked PostTask(). Ownership of |functor|
- // is passed and destroyed on |this| thread after it is invoked.
+ // is passed and (usually, see below) destroyed on |this| thread after it is
+ // invoked.
// Requirements of FunctorT:
// - FunctorT is movable.
// - FunctorT implements "T operator()()" or "T operator()() const" for some T
@@ -230,6 +248,17 @@
// after operation() has been invoked.
// - The functor must not cause the thread to quit before PostTask() is done.
//
+ // Destruction of the functor/task mimics what TaskQueue::PostTask does: If
+ // the task is run, it will be destroyed on |this| thread. However, if there
+ // are pending tasks by the time the Thread is destroyed, or a task is posted
+ // to a thread that is quitting, the task is destroyed immediately, on the
+ // calling thread. Destroying the Thread only blocks for any currently running
+ // task to complete. Note that TQ abstraction is even vaguer on how
+ // destruction happens in these cases, allowing destruction to happen
+ // asynchronously at a later time and on some arbitrary thread. So to ease
+ // migration, don't depend on Thread::PostTask destroying un-run tasks
+ // immediately.
+ //
// Example - Calling a class method:
// class Foo {
// public:
@@ -243,18 +272,12 @@
// [&x, &y] { x.TrackComputations(y.Compute()); });
template <class FunctorT>
void PostTask(const Location& posted_from, FunctorT&& functor) {
- Post(posted_from,
- new rtc_thread_internal::SingleMessageHandlerWithFunctor<FunctorT>(
+ // Allocate at first call, never deallocate.
+ static auto* const handler =
+ new rtc_thread_internal::MessageHandlerWithTask;
+ Post(posted_from, handler, 0,
+ new rtc_thread_internal::MessageWithFunctor<FunctorT>(
std::forward<FunctorT>(functor)));
- // This DCHECK guarantees that the post was successful.
- // Post() doesn't say whether it succeeded, but it will only fail if the
- // thread is quitting. DCHECKing that the thread is not quitting *after*
- // posting might yield some false positives (where the thread did in fact
- // quit, but only after posting), but if we have false positives here then
- // we have a race condition anyway.
- // TODO(https://crbug.com/webrtc/10364): When Post() returns a bool we can
- // DCHECK the result instead of inferring success from IsQuitting().
- RTC_DCHECK(!IsQuitting());
}
// From MessageQueue
