From 563d497e009ffbcb146ba5937dcf8e6779ff47a2 Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Wed, 8 Jul 2020 15:56:14 +0200 Subject: [PATCH] SimulatedTaskQueue: release lock before destroying tasks. SimulatedTaskQueue::Delete() was unintentionally holding SimulatedTaskQueue::lock_ while destroying the tasks, which led to SimulatedTimeController::lock_ getting taken. The problem is fixed by destroying the tasks outside the lock. After landing https://webrtc-review.googlesource.com/c/src/+/178818, a downstream test detected a potential deadlock between SimulatedTaskQueue and SimulatedTimeController. While the test deadlock detector did not disclose complete details, it's believed that the deadlock detector reacted because it observed another locking order than it had previously throughout the execution of the test. Bug: webrtc:11567 Change-Id: If6eafe89e2421f0c5acc6aede3419bd4fe470599 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178875 Reviewed-by: Sebastian Jansson Commit-Queue: Markus Handell Cr-Commit-Position: refs/heads/master@{#31679} --- test/time_controller/simulated_task_queue.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/time_controller/simulated_task_queue.cc b/test/time_controller/simulated_task_queue.cc index 6bc96c73b9..59fabc2292 100644 --- a/test/time_controller/simulated_task_queue.cc +++ b/test/time_controller/simulated_task_queue.cc @@ -27,11 +27,17 @@ SimulatedTaskQueue::~SimulatedTaskQueue() { } void SimulatedTaskQueue::Delete() { + // Need to destroy the tasks outside of the lock because task destruction + // can lead to re-entry in SimulatedTaskQueue via custom destructors. + std::deque> ready_tasks; + std::map>> delayed_tasks; { rtc::CritScope lock(&lock_); - ready_tasks_.clear(); - delayed_tasks_.clear(); + ready_tasks_.swap(ready_tasks); + delayed_tasks_.swap(delayed_tasks); } + ready_tasks.clear(); + delayed_tasks.clear(); delete this; }