admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

SimulatedTaskQueue: release lock before destroying tasks.

Browse Source 
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 <srte@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31679}
This commit is contained in:
Markus Handell 2020-07-08 15:56:14 +02:00 committed by Commit Bot
parent 4465ee1d50
commit 563d497e00
1 changed files with 8 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
test/time_controller/simulated_task_queue.cc
View File

@ -27,11 +27,17 @@ SimulatedTaskQueue::~SimulatedTaskQueue() {
} }
void SimulatedTaskQueue::Delete() { 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<std::unique_ptr<QueuedTask>> ready_tasks;
std::map<Timestamp, std::vector<std::unique_ptr<QueuedTask>>> delayed_tasks;
{ {
rtc::CritScope lock(&lock_); rtc::CritScope lock(&lock_);
ready_tasks_.clear(); ready_tasks_.swap(ready_tasks);
delayed_tasks_.clear(); delayed_tasks_.swap(delayed_tasks);
} }
ready_tasks.clear();
delayed_tasks.clear();
delete this; delete this;
} }