From f15fb452efe5a7082dd49faceba1ff123985f258 Mon Sep 17 00:00:00 2001 From: deadbeef Date: Sat, 25 Feb 2017 13:37:59 -0800 Subject: [PATCH] Fix occasional race in VideoCapturerTrackSource seen by memcheck bot. The issue was that VideoCapturerTrackSource was adding a reference to itself, causing it to not be deleted even after no external objects reference it. The objects underneath it (threads for instance) may then be destroyed before the object dereferences them. BUG=webrtc:6487 Review-Url: https://codereview.webrtc.org/2717023002 Cr-Commit-Position: refs/heads/master@{#16841} --- webrtc/pc/videocapturertracksource.cc | 8 ++++++-- webrtc/pc/videocapturertracksource.h | 10 +++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/webrtc/pc/videocapturertracksource.cc b/webrtc/pc/videocapturertracksource.cc index 2bb29da114..2c64b42a07 100644 --- a/webrtc/pc/videocapturertracksource.cc +++ b/webrtc/pc/videocapturertracksource.cc @@ -382,10 +382,14 @@ void VideoCapturerTrackSource::OnStateChange( cricket::VideoCapturer* capturer, cricket::CaptureState capture_state) { if (rtc::Thread::Current() != signaling_thread_) { + // Use rtc::Unretained, because we don't want this to capture a reference + // to ourselves. If our destructor is called while this task is executing, + // that's fine; our AsyncInvoker destructor will wait for it to finish if + // it isn't simply canceled. invoker_.AsyncInvoke( RTC_FROM_HERE, signaling_thread_, - rtc::Bind(&VideoCapturerTrackSource::OnStateChange, this, capturer, - capture_state)); + rtc::Bind(&VideoCapturerTrackSource::OnStateChange, + rtc::Unretained(this), capturer, capture_state)); return; } diff --git a/webrtc/pc/videocapturertracksource.h b/webrtc/pc/videocapturertracksource.h index 1a2529b1b6..2477340cdf 100644 --- a/webrtc/pc/videocapturertracksource.h +++ b/webrtc/pc/videocapturertracksource.h @@ -47,14 +47,10 @@ class VideoCapturerTrackSource : public VideoTrackSource, std::unique_ptr capturer, bool remote); - bool is_screencast() const override { - return video_capturer_->IsScreencast(); - } - rtc::Optional needs_denoising() const override { - return needs_denoising_; - } + bool is_screencast() const final { return video_capturer_->IsScreencast(); } + rtc::Optional needs_denoising() const final { return needs_denoising_; } - bool GetStats(Stats* stats) override; + bool GetStats(Stats* stats) final; protected: VideoCapturerTrackSource(rtc::Thread* worker_thread,