From 8ff860a35d611a976d94a18cb0778bf96b704427 Mon Sep 17 00:00:00 2001 From: perkj Date: Mon, 3 Oct 2016 00:30:04 -0700 Subject: [PATCH] Add support for WeakPtr The implementation is borrowed from Chromium. Also change use of raw pointers in VideoSendStreamImpl to use WeakPtr<> BUG= webrtc:6289 Review-Url: https://codereview.webrtc.org/2367853002 Cr-Commit-Position: refs/heads/master@{#14468} --- webrtc/BUILD.gn | 1 + webrtc/base/BUILD.gn | 2 + webrtc/base/base.gyp | 2 + webrtc/base/weak_ptr.cc | 87 ++++++++++ webrtc/base/weak_ptr.h | 272 ++++++++++++++++++++++++++++++ webrtc/base/weak_ptr_unittest.cc | 233 +++++++++++++++++++++++++ webrtc/video/video_send_stream.cc | 42 +++-- 7 files changed, 622 insertions(+), 17 deletions(-) create mode 100644 webrtc/base/weak_ptr.cc create mode 100644 webrtc/base/weak_ptr.h create mode 100644 webrtc/base/weak_ptr_unittest.cc diff --git a/webrtc/BUILD.gn b/webrtc/BUILD.gn index 92949cf0cb..aaea4e3b9d 100644 --- a/webrtc/BUILD.gn +++ b/webrtc/BUILD.gn @@ -439,6 +439,7 @@ if (rtc_include_tests) { "base/timeutils_unittest.cc", "base/urlencode_unittest.cc", "base/versionparsing_unittest.cc", + "base/weak_ptr_unittest.cc", # TODO(ronghuawu): Reenable this test. # "windowpicker_unittest.cc", diff --git a/webrtc/base/BUILD.gn b/webrtc/base/BUILD.gn index 72564148a7..b8c0a5b2a3 100644 --- a/webrtc/base/BUILD.gn +++ b/webrtc/base/BUILD.gn @@ -210,6 +210,8 @@ rtc_static_library("rtc_task_queue") { "sequenced_task_checker.h", "sequenced_task_checker_impl.cc", "sequenced_task_checker_impl.h", + "weak_ptr.cc", + "weak_ptr.h", ] if (build_with_chromium) { diff --git a/webrtc/base/base.gyp b/webrtc/base/base.gyp index 0e923a308a..3609de951e 100644 --- a/webrtc/base/base.gyp +++ b/webrtc/base/base.gyp @@ -167,6 +167,8 @@ 'sequenced_task_checker.h', 'sequenced_task_checker_impl.cc', 'sequenced_task_checker_impl.h', + 'weak_ptr.cc', + 'weak_ptr.h', ], 'conditions': [ ['build_with_chromium==1', { diff --git a/webrtc/base/weak_ptr.cc b/webrtc/base/weak_ptr.cc new file mode 100644 index 0000000000..4eef7eacc5 --- /dev/null +++ b/webrtc/base/weak_ptr.cc @@ -0,0 +1,87 @@ +/* + * Copyright 2016 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "webrtc/base/weak_ptr.h" + +// The implementation is borrowed from chromium except that it does not +// implement SupportsWeakPtr. + +namespace rtc { +namespace internal { + +WeakReference::Flag::Flag() : is_valid_(true) { + // Flags only become bound when checked for validity, or invalidated, + // so that we can check that later validity/invalidation operations on + // the same Flag take place on the same sequence. + checker_.Detach(); +} + +void WeakReference::Flag::Invalidate() { + RTC_DCHECK(checker_.CalledSequentially()) + << "WeakPtrs must be invalidated on the same sequence."; + is_valid_ = false; +} + +bool WeakReference::Flag::IsValid() const { + RTC_DCHECK(checker_.CalledSequentially()) + << "WeakPtrs must be checked on the same sequence."; + return is_valid_; +} + +WeakReference::Flag::~Flag() {} + +WeakReference::WeakReference() {} + +WeakReference::WeakReference(const Flag* flag) : flag_(flag) {} + +WeakReference::~WeakReference() {} + +WeakReference::WeakReference(WeakReference&& other) = default; + +WeakReference::WeakReference(const WeakReference& other) = default; + +bool WeakReference::is_valid() const { + return flag_.get() && flag_->IsValid(); +} + +WeakReferenceOwner::WeakReferenceOwner() { + checker_.Detach(); +} + +WeakReferenceOwner::~WeakReferenceOwner() { + RTC_DCHECK(checker_.CalledSequentially()); + Invalidate(); +} + +WeakReference WeakReferenceOwner::GetRef() const { + RTC_DCHECK(checker_.CalledSequentially()); + // If we hold the last reference to the Flag then create a new one. + if (!HasRefs()) + flag_ = new RefCountedObject(); + + return WeakReference(flag_.get()); +} + +void WeakReferenceOwner::Invalidate() { + RTC_DCHECK(checker_.CalledSequentially()); + if (flag_.get()) { + flag_->Invalidate(); + flag_ = NULL; + } +} + +WeakPtrBase::WeakPtrBase() {} + +WeakPtrBase::~WeakPtrBase() {} + +WeakPtrBase::WeakPtrBase(const WeakReference& ref) : ref_(ref) {} + +} // namespace internal +} // namespace rtc diff --git a/webrtc/base/weak_ptr.h b/webrtc/base/weak_ptr.h new file mode 100644 index 0000000000..28789d014b --- /dev/null +++ b/webrtc/base/weak_ptr.h @@ -0,0 +1,272 @@ +/* + * Copyright 2016 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef WEBRTC_BASE_WEAK_PTR_H_ +#define WEBRTC_BASE_WEAK_PTR_H_ + +#include + +#include + +#include "webrtc/base/refcount.h" +#include "webrtc/base/scoped_ref_ptr.h" +#include "webrtc/base/sequenced_task_checker.h" + +// The implementation is borrowed from chromium except that it does not +// implement SupportsWeakPtr. + +// Weak pointers are pointers to an object that do not affect its lifetime, +// and which may be invalidated (i.e. reset to nullptr) by the object, or its +// owner, at any time, most commonly when the object is about to be deleted. + +// Weak pointers are useful when an object needs to be accessed safely by one +// or more objects other than its owner, and those callers can cope with the +// object vanishing and e.g. tasks posted to it being silently dropped. +// Reference-counting such an object would complicate the ownership graph and +// make it harder to reason about the object's lifetime. + +// EXAMPLE: +// +// class Controller { +// public: +// Controller() : weak_factory_(this) {} +// void SpawnWorker() { Worker::StartNew(weak_factory_.GetWeakPtr()); } +// void WorkComplete(const Result& result) { ... } +// private: +// // Member variables should appear before the WeakPtrFactory, to ensure +// // that any WeakPtrs to Controller are invalidated before its members +// // variable's destructors are executed, rendering them invalid. +// WeakPtrFactory weak_factory_; +// }; +// +// class Worker { +// public: +// static void StartNew(const WeakPtr& controller) { +// Worker* worker = new Worker(controller); +// // Kick off asynchronous processing... +// } +// private: +// Worker(const WeakPtr& controller) +// : controller_(controller) {} +// void DidCompleteAsynchronousProcessing(const Result& result) { +// if (controller_) +// controller_->WorkComplete(result); +// } +// WeakPtr controller_; +// }; +// +// With this implementation a caller may use SpawnWorker() to dispatch multiple +// Workers and subsequently delete the Controller, without waiting for all +// Workers to have completed. + +// ------------------------- IMPORTANT: Thread-safety ------------------------- + +// Weak pointers may be passed safely between threads, but must always be +// dereferenced and invalidated on the same TaskQueue or thread, otherwise +// checking the pointer would be racey. +// +// To ensure correct use, the first time a WeakPtr issued by a WeakPtrFactory +// is dereferenced, the factory and its WeakPtrs become bound to the calling +// TaskQueue/thread, and cannot be dereferenced or +// invalidated on any other TaskQueue/thread. Bound WeakPtrs can still be handed +// off to other TaskQueues, e.g. to use to post tasks back to object on the +// bound sequence. +// +// Thus, at least one WeakPtr object must exist and have been dereferenced on +// the correct thread to enforce that other WeakPtr objects will enforce they +// are used on the desired thread. + +namespace rtc { + +namespace internal { + +class WeakReference { + public: + // Although Flag is bound to a specific sequence, it may be + // deleted from another via base::WeakPtr::~WeakPtr(). + class Flag : public RefCountInterface { + public: + Flag(); + + void Invalidate(); + bool IsValid() const; + + private: + friend class RefCountedObject; + + ~Flag() override; + + SequencedTaskChecker checker_; + bool is_valid_; + }; + + WeakReference(); + explicit WeakReference(const Flag* flag); + ~WeakReference(); + + WeakReference(WeakReference&& other); + WeakReference(const WeakReference& other); + WeakReference& operator=(WeakReference&& other) = default; + WeakReference& operator=(const WeakReference& other) = default; + + bool is_valid() const; + + private: + scoped_refptr flag_; +}; + +class WeakReferenceOwner { + public: + WeakReferenceOwner(); + ~WeakReferenceOwner(); + + WeakReference GetRef() const; + + bool HasRefs() const { return flag_.get() && !flag_->HasOneRef(); } + + void Invalidate(); + + private: + SequencedTaskChecker checker_; + mutable scoped_refptr> flag_; +}; + +// This class simplifies the implementation of WeakPtr's type conversion +// constructor by avoiding the need for a public accessor for ref_. A +// WeakPtr cannot access the private members of WeakPtr, so this +// base class gives us a way to access ref_ in a protected fashion. +class WeakPtrBase { + public: + WeakPtrBase(); + ~WeakPtrBase(); + + WeakPtrBase(const WeakPtrBase& other) = default; + WeakPtrBase(WeakPtrBase&& other) = default; + WeakPtrBase& operator=(const WeakPtrBase& other) = default; + WeakPtrBase& operator=(WeakPtrBase&& other) = default; + + protected: + explicit WeakPtrBase(const WeakReference& ref); + + WeakReference ref_; +}; + +} // namespace internal + +template +class WeakPtrFactory; + +template +class WeakPtr : public internal::WeakPtrBase { + public: + WeakPtr() : ptr_(nullptr) {} + + // Allow conversion from U to T provided U "is a" T. Note that this + // is separate from the (implicit) copy and move constructors. + template + WeakPtr(const WeakPtr& other) + : internal::WeakPtrBase(other), ptr_(other.ptr_) {} + template + WeakPtr(WeakPtr&& other) + : internal::WeakPtrBase(std::move(other)), ptr_(other.ptr_) {} + + T* get() const { return ref_.is_valid() ? ptr_ : nullptr; } + + T& operator*() const { + RTC_DCHECK(get() != nullptr); + return *get(); + } + T* operator->() const { + RTC_DCHECK(get() != nullptr); + return get(); + } + + void reset() { + ref_ = internal::WeakReference(); + ptr_ = nullptr; + } + + // Allow conditionals to test validity, e.g. if (weak_ptr) {...}; + explicit operator bool() const { return get() != nullptr; } + + private: + template + friend class WeakPtr; + friend class WeakPtrFactory; + + WeakPtr(const internal::WeakReference& ref, T* ptr) + : internal::WeakPtrBase(ref), ptr_(ptr) {} + + // This pointer is only valid when ref_.is_valid() is true. Otherwise, its + // value is undefined (as opposed to nullptr). + T* ptr_; +}; + +// Allow callers to compare WeakPtrs against nullptr to test validity. +template +bool operator!=(const WeakPtr& weak_ptr, std::nullptr_t) { + return !(weak_ptr == nullptr); +} +template +bool operator!=(std::nullptr_t, const WeakPtr& weak_ptr) { + return weak_ptr != nullptr; +} +template +bool operator==(const WeakPtr& weak_ptr, std::nullptr_t) { + return weak_ptr.get() == nullptr; +} +template +bool operator==(std::nullptr_t, const WeakPtr& weak_ptr) { + return weak_ptr == nullptr; +} + +// A class may be composed of a WeakPtrFactory and thereby +// control how it exposes weak pointers to itself. This is helpful if you only +// need weak pointers within the implementation of a class. This class is also +// useful when working with primitive types. For example, you could have a +// WeakPtrFactory that is used to pass around a weak reference to a bool. + +// Note that GetWeakPtr must be called on one and only one TaskQueue or thread +// and the WeakPtr must only be dereferenced and invalidated on that same +// TaskQueue/thread. A WeakPtr instance can be copied and posted to other +// sequences though as long as it is not dereferenced (WeakPtr::get()). +template +class WeakPtrFactory { + public: + explicit WeakPtrFactory(T* ptr) : ptr_(ptr) {} + + ~WeakPtrFactory() { ptr_ = nullptr; } + + WeakPtr GetWeakPtr() { + RTC_DCHECK(ptr_); + return WeakPtr(weak_reference_owner_.GetRef(), ptr_); + } + + // Call this method to invalidate all existing weak pointers. + void InvalidateWeakPtrs() { + RTC_DCHECK(ptr_); + weak_reference_owner_.Invalidate(); + } + + // Call this method to determine if any weak pointers exist. + bool HasWeakPtrs() const { + RTC_DCHECK(ptr_); + return weak_reference_owner_.HasRefs(); + } + + private: + internal::WeakReferenceOwner weak_reference_owner_; + T* ptr_; + RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(WeakPtrFactory); +}; + +} // namespace rtc + +#endif // WEBRTC_BASE_WEAK_PTR_H_ diff --git a/webrtc/base/weak_ptr_unittest.cc b/webrtc/base/weak_ptr_unittest.cc new file mode 100644 index 0000000000..a3cd1f839c --- /dev/null +++ b/webrtc/base/weak_ptr_unittest.cc @@ -0,0 +1,233 @@ +/* + * Copyright 2016 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include + +#include "webrtc/base/gunit.h" +#include "webrtc/base/task_queue.h" +#include "webrtc/base/weak_ptr.h" + +namespace rtc { + +namespace { + +struct Base { + std::string member; +}; +struct Derived : public Base {}; + +struct Target {}; + +struct Arrow { + WeakPtr target; +}; + +struct TargetWithFactory : public Target { + TargetWithFactory() : factory(this) {} + WeakPtrFactory factory; +}; + +} // namespace + +TEST(WeakPtrFactoryTest, Basic) { + int data; + WeakPtrFactory factory(&data); + WeakPtr ptr = factory.GetWeakPtr(); + EXPECT_EQ(&data, ptr.get()); +} + +TEST(WeakPtrFactoryTest, Comparison) { + int data; + WeakPtrFactory factory(&data); + WeakPtr ptr = factory.GetWeakPtr(); + WeakPtr ptr2 = ptr; + EXPECT_EQ(ptr.get(), ptr2.get()); +} + +TEST(WeakPtrFactoryTest, Move) { + int data; + WeakPtrFactory factory(&data); + WeakPtr ptr = factory.GetWeakPtr(); + WeakPtr ptr2 = factory.GetWeakPtr(); + WeakPtr ptr3 = std::move(ptr2); + EXPECT_NE(ptr.get(), ptr2.get()); + EXPECT_EQ(ptr.get(), ptr3.get()); +} + +TEST(WeakPtrFactoryTest, OutOfScope) { + WeakPtr ptr; + EXPECT_EQ(nullptr, ptr.get()); + { + int data; + WeakPtrFactory factory(&data); + ptr = factory.GetWeakPtr(); + EXPECT_EQ(&data, ptr.get()); + } + EXPECT_EQ(nullptr, ptr.get()); +} + +TEST(WeakPtrFactoryTest, Multiple) { + WeakPtr a, b; + { + int data; + WeakPtrFactory factory(&data); + a = factory.GetWeakPtr(); + b = factory.GetWeakPtr(); + EXPECT_EQ(&data, a.get()); + EXPECT_EQ(&data, b.get()); + } + EXPECT_EQ(nullptr, a.get()); + EXPECT_EQ(nullptr, b.get()); +} + +TEST(WeakPtrFactoryTest, MultipleStaged) { + WeakPtr a; + { + int data; + WeakPtrFactory factory(&data); + a = factory.GetWeakPtr(); + { WeakPtr b = factory.GetWeakPtr(); } + EXPECT_NE(nullptr, a.get()); + } + EXPECT_EQ(nullptr, a.get()); +} + +TEST(WeakPtrFactoryTest, Dereference) { + Base data; + data.member = "123456"; + WeakPtrFactory factory(&data); + WeakPtr ptr = factory.GetWeakPtr(); + EXPECT_EQ(&data, ptr.get()); + EXPECT_EQ(data.member, (*ptr).member); + EXPECT_EQ(data.member, ptr->member); +} + +TEST(WeakPtrFactoryTest, UpCast) { + Derived data; + WeakPtrFactory factory(&data); + WeakPtr ptr = factory.GetWeakPtr(); + ptr = factory.GetWeakPtr(); + EXPECT_EQ(ptr.get(), &data); +} + +TEST(WeakPtrTest, DefaultConstructor) { + WeakPtr ptr; + EXPECT_EQ(nullptr, ptr.get()); +} + +TEST(WeakPtrFactoryTest, BooleanTesting) { + int data; + WeakPtrFactory factory(&data); + + WeakPtr ptr_to_an_instance = factory.GetWeakPtr(); + EXPECT_TRUE(ptr_to_an_instance); + EXPECT_FALSE(!ptr_to_an_instance); + + if (ptr_to_an_instance) { + } else { + ADD_FAILURE() << "Pointer to an instance should result in true."; + } + + if (!ptr_to_an_instance) { // check for operator!(). + ADD_FAILURE() << "Pointer to an instance should result in !x being false."; + } + + WeakPtr null_ptr; + EXPECT_FALSE(null_ptr); + EXPECT_TRUE(!null_ptr); + + if (null_ptr) { + ADD_FAILURE() << "Null pointer should result in false."; + } + + if (!null_ptr) { // check for operator!(). + } else { + ADD_FAILURE() << "Null pointer should result in !x being true."; + } +} + +TEST(WeakPtrFactoryTest, ComparisonToNull) { + int data; + WeakPtrFactory factory(&data); + + WeakPtr ptr_to_an_instance = factory.GetWeakPtr(); + EXPECT_NE(nullptr, ptr_to_an_instance); + EXPECT_NE(ptr_to_an_instance, nullptr); + + WeakPtr null_ptr; + EXPECT_EQ(null_ptr, nullptr); + EXPECT_EQ(nullptr, null_ptr); +} + +TEST(WeakPtrTest, InvalidateWeakPtrs) { + int data; + WeakPtrFactory factory(&data); + WeakPtr ptr = factory.GetWeakPtr(); + EXPECT_EQ(&data, ptr.get()); + EXPECT_TRUE(factory.HasWeakPtrs()); + factory.InvalidateWeakPtrs(); + EXPECT_EQ(nullptr, ptr.get()); + EXPECT_FALSE(factory.HasWeakPtrs()); + + // Test that the factory can create new weak pointers after a + // InvalidateWeakPtrs call, and they remain valid until the next + // InvalidateWeakPtrs call. + WeakPtr ptr2 = factory.GetWeakPtr(); + EXPECT_EQ(&data, ptr2.get()); + EXPECT_TRUE(factory.HasWeakPtrs()); + factory.InvalidateWeakPtrs(); + EXPECT_EQ(nullptr, ptr2.get()); + EXPECT_FALSE(factory.HasWeakPtrs()); +} + +TEST(WeakPtrTest, HasWeakPtrs) { + int data; + WeakPtrFactory factory(&data); + { + WeakPtr ptr = factory.GetWeakPtr(); + EXPECT_TRUE(factory.HasWeakPtrs()); + } + EXPECT_FALSE(factory.HasWeakPtrs()); +} + +template +std::unique_ptr NewObjectCreatedOnTaskQueue() { + std::unique_ptr obj; + TaskQueue queue("NewObjectCreatedOnTaskQueue"); + Event event(false, false); + queue.PostTask([&event, &obj] { + obj.reset(new T()); + event.Set(); + }); + EXPECT_TRUE(event.Wait(1000)); + return obj; +} + +TEST(WeakPtrTest, ObjectAndWeakPtrOnDifferentThreads) { + // Test that it is OK to create an object with a WeakPtrFactory one thread, + // but use it on another. This tests that we do not trip runtime checks that + // ensure that a WeakPtr is not used by multiple threads. + std::unique_ptr target( + NewObjectCreatedOnTaskQueue()); + WeakPtr weak_ptr = target->factory.GetWeakPtr(); + EXPECT_EQ(target.get(), weak_ptr.get()); +} + +TEST(WeakPtrTest, WeakPtrInitiateAndUseOnDifferentThreads) { + // Test that it is OK to create an object that has a WeakPtr member on one + // thread, but use it on another. This tests that we do not trip runtime + // checks that ensure that a WeakPtr is not used by multiple threads. + std::unique_ptr arrow(NewObjectCreatedOnTaskQueue()); + TargetWithFactory target; + arrow->target = target.factory.GetWeakPtr(); + EXPECT_EQ(&target, arrow->target.get()); +} + +} // namespace rtc diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 2a727372af..3ff85d4f63 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -20,6 +20,7 @@ #include "webrtc/base/file.h" #include "webrtc/base/logging.h" #include "webrtc/base/trace_event.h" +#include "webrtc/base/weak_ptr.h" #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" #include "webrtc/modules/congestion_controller/include/congestion_controller.h" #include "webrtc/modules/pacing/packet_router.h" @@ -339,6 +340,14 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, // RtpRtcp modules, declared here as they use other members on construction. const std::vector rtp_rtcp_modules_; PayloadRouter payload_router_; + + // |weak_ptr_| to our self. This is used since we can not call + // |weak_ptr_factory_.GetWeakPtr| from multiple sequences but it is ok to copy + // an existing WeakPtr. + rtc::WeakPtr weak_ptr_; + // |weak_ptr_factory_| must be declared last to make sure all WeakPtr's are + // invalidated before any other members are destroyed. + rtc::WeakPtrFactory weak_ptr_factory_; }; // TODO(tommi): See if there's a more elegant way to create a task that creates @@ -431,12 +440,13 @@ class VideoSendStream::DestructAndGetRtpStateTask : public rtc::QueuedTask { class VideoSendStreamImpl::CheckEncoderActivityTask : public rtc::QueuedTask { public: static const int kEncoderTimeOutMs = 2000; - explicit CheckEncoderActivityTask(VideoSendStreamImpl* send_stream) - : activity_(0), send_stream_(send_stream), timed_out_(false) {} + explicit CheckEncoderActivityTask( + const rtc::WeakPtr& send_stream) + : activity_(0), send_stream_(std::move(send_stream)), timed_out_(false) {} void Stop() { RTC_CHECK(task_checker_.CalledSequentially()); - send_stream_ = nullptr; + send_stream_.reset(); } void UpdateEncoderActivity() { @@ -472,27 +482,28 @@ class VideoSendStreamImpl::CheckEncoderActivityTask : public rtc::QueuedTask { volatile int activity_; rtc::SequencedTaskChecker task_checker_; - VideoSendStreamImpl* send_stream_; + rtc::WeakPtr send_stream_; bool timed_out_; }; class VideoSendStreamImpl::EncoderReconfiguredTask : public rtc::QueuedTask { public: - EncoderReconfiguredTask(VideoSendStreamImpl* send_stream, + EncoderReconfiguredTask(const rtc::WeakPtr& send_stream, std::vector streams, int min_transmit_bitrate_bps) - : send_stream_(send_stream), + : send_stream_(std::move(send_stream)), streams_(std::move(streams)), min_transmit_bitrate_bps_(min_transmit_bitrate_bps) {} private: bool Run() override { - send_stream_->OnEncoderConfigurationChanged(std::move(streams_), - min_transmit_bitrate_bps_); + if (send_stream_) + send_stream_->OnEncoderConfigurationChanged(std::move(streams_), + min_transmit_bitrate_bps_); return true; } - VideoSendStreamImpl* send_stream_; + rtc::WeakPtr send_stream_; std::vector streams_; int min_transmit_bitrate_bps_; }; @@ -665,9 +676,11 @@ VideoSendStreamImpl::VideoSendStreamImpl( congestion_controller_->GetRetransmissionRateLimiter(), config_->rtp.ssrcs.size())), payload_router_(rtp_rtcp_modules_, - config_->encoder_settings.payload_type) { + config_->encoder_settings.payload_type), + weak_ptr_factory_(this) { RTC_DCHECK_RUN_ON(worker_queue_); LOG(LS_INFO) << "VideoSendStreamInternal: " << config_->ToString(); + weak_ptr_ = weak_ptr_factory_.GetWeakPtr(); module_process_thread_checker_.DetachFromThread(); RTC_DCHECK(!config_->rtp.ssrcs.empty()); @@ -778,7 +791,7 @@ void VideoSendStreamImpl::Start() { { rtc::CritScope lock(&encoder_activity_crit_sect_); RTC_DCHECK(!check_encoder_activity_task_); - check_encoder_activity_task_ = new CheckEncoderActivityTask(this); + check_encoder_activity_task_ = new CheckEncoderActivityTask(weak_ptr_); worker_queue_->PostDelayedTask( std::unique_ptr(check_encoder_activity_task_), CheckEncoderActivityTask::kEncoderTimeOutMs); @@ -827,14 +840,9 @@ void VideoSendStreamImpl::OnEncoderConfigurationChanged( std::vector streams, int min_transmit_bitrate_bps) { if (!worker_queue_->IsCurrent()) { - // TODO(perkj): Using |this| in post is safe for now since destruction of - // VideoSendStreamImpl is synchronized in - // VideoSendStream::StopPermanentlyAndGetRtpStates. But we should really - // use some kind of weak_ptr to guarantee that VideoSendStreamImpl is still - // alive when this task runs. worker_queue_->PostTask( std::unique_ptr(new EncoderReconfiguredTask( - this, std::move(streams), min_transmit_bitrate_bps))); + weak_ptr_, std::move(streams), min_transmit_bitrate_bps))); return; } RTC_DCHECK_GE(config_->rtp.ssrcs.size(), streams.size());