Lock once in CallbackTaskManager::Flush()
This patch ensures there will only be one lock in
CallbackTaskManager::Flush() instead of two (in IsEmpty() and in
AcquireCallbackTasks()).
Bug: dawn:752
Change-Id: I3a33557fde65fcb414ee0a349d5518d0858ca762
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/151681
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/src/dawn/native/CallbackTaskManager.cpp b/src/dawn/native/CallbackTaskManager.cpp
index ea72b96..7da85c2 100644
--- a/src/dawn/native/CallbackTaskManager.cpp
+++ b/src/dawn/native/CallbackTaskManager.cpp
@@ -71,14 +71,6 @@
return mCallbackTaskQueue.empty();
}
-std::vector<std::unique_ptr<CallbackTask>> CallbackTaskManager::AcquireCallbackTasks() {
- std::lock_guard<std::mutex> lock(mCallbackTaskQueueMutex);
-
- std::vector<std::unique_ptr<CallbackTask>> allTasks;
- allTasks.swap(mCallbackTaskQueue);
- return allTasks;
-}
-
void CallbackTaskManager::AddCallbackTask(std::unique_ptr<CallbackTask> callbackTask) {
std::lock_guard<std::mutex> lock(mCallbackTaskQueueMutex);
mCallbackTaskQueue.push_back(std::move(callbackTask));
@@ -103,15 +95,21 @@
}
void CallbackTaskManager::Flush() {
- if (!IsEmpty()) {
- // If a user calls Queue::Submit inside the callback, then the device will be ticked,
- // which in turns ticks the tracker, causing reentrance and dead lock here. To prevent
- // such reentrant call, we remove all the callback tasks from mCallbackTaskManager,
- // update mCallbackTaskManager, then call all the callbacks.
- auto callbackTasks = AcquireCallbackTasks();
- for (std::unique_ptr<CallbackTask>& callbackTask : callbackTasks) {
- callbackTask->Execute();
- }
+ std::unique_lock<std::mutex> lock(mCallbackTaskQueueMutex);
+ if (mCallbackTaskQueue.empty()) {
+ return;
+ }
+
+ // If a user calls Queue::Submit inside the callback, then the device will be ticked,
+ // which in turns ticks the tracker, causing reentrance and dead lock here. To prevent
+ // such reentrant call, we remove all the callback tasks from mCallbackTaskManager,
+ // update mCallbackTaskManager, then call all the callbacks.
+ std::vector<std::unique_ptr<CallbackTask>> allTasks;
+ allTasks.swap(mCallbackTaskQueue);
+ lock.unlock();
+
+ for (std::unique_ptr<CallbackTask>& callbackTask : allTasks) {
+ callbackTask->Execute();
}
}
diff --git a/src/dawn/native/CallbackTaskManager.h b/src/dawn/native/CallbackTaskManager.h
index 3cf0091..06211be 100644
--- a/src/dawn/native/CallbackTaskManager.h
+++ b/src/dawn/native/CallbackTaskManager.h
@@ -67,8 +67,6 @@
void Flush();
private:
- std::vector<std::unique_ptr<CallbackTask>> AcquireCallbackTasks();
-
std::mutex mCallbackTaskQueueMutex;
std::vector<std::unique_ptr<CallbackTask>> mCallbackTaskQueue;
};
