From ad82a424e4fe00b0b64dd6886f16a5bf68424515 Mon Sep 17 00:00:00 2001 From: Elad Alon Date: Mon, 3 Dec 2018 12:48:16 +0100 Subject: [PATCH] Fix race over RtcEventLogImpl::task_queue_ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RtcEventLogImpl::task_queue_ is a std::unique_ptr. When a unique_ptr is destroyed, it first sets its internal pointer to point to null, and only then invokes the destructor of that object. However, the code in RtcEventLogImpl relies on rtc::TaskQueue's property, that its destructor blocks on executing tasks. We solve by manually invoking the destructor, and only resetting the internal pointer thereafter. In theory, we could have changed the unique_ptr to a raw pointer at this point. We avoid that, so as to keep the ownership clearer to readers of the code. Bug: webrtc:10085 Change-Id: I54bbf5d6bae019757ca2e31ee960d558058ccc42 Reviewed-on: https://webrtc-review.googlesource.com/c/112598 Commit-Queue: Elad Alon Reviewed-by: Björn Terelius Cr-Commit-Position: refs/heads/master@{#25875} --- logging/rtc_event_log/rtc_event_log_impl.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/logging/rtc_event_log/rtc_event_log_impl.cc b/logging/rtc_event_log/rtc_event_log_impl.cc index c022a3d418..ac2d918373 100644 --- a/logging/rtc_event_log/rtc_event_log_impl.cc +++ b/logging/rtc_event_log/rtc_event_log_impl.cc @@ -159,6 +159,12 @@ RtcEventLogImpl::~RtcEventLogImpl() { // If we're logging to the output, this will stop that. Blocking function. StopLogging(); + + // We want to block on any executing task by invoking ~TaskQueue() before + // we set unique_ptr's internal pointer to null. + rtc::TaskQueue* tq = task_queue_.get(); + delete tq; + task_queue_.release(); } bool RtcEventLogImpl::StartLogging(std::unique_ptr output,