Make implicit resource destruction thread-safe
- synchronize access to device memory allocators,
deletion queues, and residency manager
- Remove IsAlive check from ApiObjectBase::Destroy.
Untrack already checks if an object is live and removes
it from the list in a thread-safe way.
- Add basic atomics to ExecutionQueue serials.
They may be read during object deletion concurrently
while they are updated.
Bug: dawn:2069
Change-Id: I48a2121ca27dfbab1db97303b80b1a0039880483
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/153481
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Loko Kung <lokokung@google.com>
diff --git a/src/dawn/native/Buffer.cpp b/src/dawn/native/Buffer.cpp
index c88da57..f90786d 100644
--- a/src/dawn/native/Buffer.cpp
+++ b/src/dawn/native/Buffer.cpp
@@ -239,6 +239,13 @@
}
void BufferBase::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the buffer is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the buffer.
+ // - It may be called when the last ref to the buffer is dropped and the buffer
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the buffer since there are no other live refs.
if (mState == BufferState::Mapped || mState == BufferState::PendingMap) {
UnmapInternal(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
} else if (mState == BufferState::MappedAtCreation) {
diff --git a/src/dawn/native/ExecutionQueue.cpp b/src/dawn/native/ExecutionQueue.cpp
index 0aa1518..198ae95 100644
--- a/src/dawn/native/ExecutionQueue.cpp
+++ b/src/dawn/native/ExecutionQueue.cpp
@@ -17,11 +17,11 @@
namespace dawn::native {
ExecutionSerial ExecutionQueueBase::GetPendingCommandSerial() const {
- return mLastSubmittedSerial + ExecutionSerial(1);
+ return ExecutionSerial(mLastSubmittedSerial.load(std::memory_order_acquire) + 1);
}
ExecutionSerial ExecutionQueueBase::GetLastSubmittedCommandSerial() const {
- return mLastSubmittedSerial;
+ return ExecutionSerial(mLastSubmittedSerial.load(std::memory_order_acquire));
}
ExecutionSerial ExecutionQueueBase::GetCompletedCommandSerial() const {
@@ -32,7 +32,8 @@
ExecutionSerial completedSerial;
DAWN_TRY_ASSIGN(completedSerial, CheckAndUpdateCompletedSerials());
- DAWN_ASSERT(completedSerial <= mLastSubmittedSerial);
+ DAWN_ASSERT(completedSerial <=
+ ExecutionSerial(mLastSubmittedSerial.load(std::memory_order_acquire)));
// completedSerial should not be less than mCompletedSerial unless it is 0.
// It can be 0 when there's no fences to check.
DAWN_ASSERT(completedSerial >= mCompletedSerial || completedSerial == ExecutionSerial(0));
@@ -46,16 +47,18 @@
void ExecutionQueueBase::AssumeCommandsComplete() {
// Bump serials so any pending callbacks can be fired.
- mLastSubmittedSerial++;
- mCompletedSerial = mLastSubmittedSerial;
+ uint64_t prev = mLastSubmittedSerial.fetch_add(1u, std::memory_order_release);
+ mCompletedSerial = ExecutionSerial(prev + 1);
}
void ExecutionQueueBase::IncrementLastSubmittedCommandSerial() {
- mLastSubmittedSerial++;
+ mLastSubmittedSerial.fetch_add(1u, std::memory_order_release);
}
bool ExecutionQueueBase::HasScheduledCommands() const {
- return mLastSubmittedSerial > mCompletedSerial || HasPendingCommands();
+ return ExecutionSerial(mLastSubmittedSerial.load(std::memory_order_acquire)) >
+ mCompletedSerial ||
+ HasPendingCommands();
}
// All prevously submitted works at the moment will supposedly complete at this serial.
diff --git a/src/dawn/native/ExecutionQueue.h b/src/dawn/native/ExecutionQueue.h
index 3c29a08..80664fb 100644
--- a/src/dawn/native/ExecutionQueue.h
+++ b/src/dawn/native/ExecutionQueue.h
@@ -15,6 +15,8 @@
#ifndef SRC_DAWN_NATIVE_EXECUTIONQUEUE_H_
#define SRC_DAWN_NATIVE_EXECUTIONQUEUE_H_
+#include <atomic>
+
#include "dawn/native/Error.h"
#include "dawn/native/IntegerTypes.h"
@@ -22,6 +24,8 @@
// Represents an engine which processes a stream of GPU work. It handles the tracking and
// update of the various ExecutionSerials related to that work.
+// TODO(dawn:831, dawn:1413): Make usage of the ExecutionQueue thread-safe. Right now it is
+// only partially safe - where observation of the last-submitted and pending serials is atomic.
class ExecutionQueueBase {
public:
// The latest serial known to have completed execution on the queue.
@@ -70,7 +74,7 @@
// During device removal, the serials could be artificially incremented
// to make it appear as if commands have been compeleted.
ExecutionSerial mCompletedSerial = kBeginningOfGPUTime;
- ExecutionSerial mLastSubmittedSerial = kBeginningOfGPUTime;
+ std::atomic<uint64_t> mLastSubmittedSerial = static_cast<uint64_t>(kBeginningOfGPUTime);
// Indicates whether the backend has pending commands to be submitted as soon as possible.
virtual bool HasPendingCommands() const = 0;
diff --git a/src/dawn/native/ExternalTexture.cpp b/src/dawn/native/ExternalTexture.cpp
index 1f64fdf..3a296a2 100644
--- a/src/dawn/native/ExternalTexture.cpp
+++ b/src/dawn/native/ExternalTexture.cpp
@@ -368,6 +368,13 @@
}
void ExternalTextureBase::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the texture is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the texture.
+ // - It may be called when the last ref to the texture is dropped and the texture
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the texture since there are no other live refs.
mState = ExternalTextureState::Destroyed;
}
diff --git a/src/dawn/native/ObjectBase.cpp b/src/dawn/native/ObjectBase.cpp
index 3f7f11c..974ad10 100644
--- a/src/dawn/native/ObjectBase.cpp
+++ b/src/dawn/native/ObjectBase.cpp
@@ -136,9 +136,6 @@
}
void ApiObjectBase::Destroy() {
- if (!IsAlive()) {
- return;
- }
ApiObjectList* list = GetObjectTrackingList();
DAWN_ASSERT(list != nullptr);
if (list->Untrack(this)) {
diff --git a/src/dawn/native/QuerySet.cpp b/src/dawn/native/QuerySet.cpp
index a58f85f..6e76615 100644
--- a/src/dawn/native/QuerySet.cpp
+++ b/src/dawn/native/QuerySet.cpp
@@ -124,6 +124,13 @@
}
void QuerySetBase::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the query set is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the query set.
+ // - It may be called when the last ref to the query set is dropped and it
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the query set since there are no other live refs.
mState = QuerySetState::Destroyed;
}
diff --git a/src/dawn/native/Queue.cpp b/src/dawn/native/Queue.cpp
index 6c1c3ca..183a92b 100644
--- a/src/dawn/native/Queue.cpp
+++ b/src/dawn/native/Queue.cpp
@@ -238,7 +238,7 @@
: ApiObjectBase(device, tag, label) {}
QueueBase::~QueueBase() {
- DAWN_ASSERT(mTasksInFlight.Empty());
+ DAWN_ASSERT(mTasksInFlight->Empty());
}
void QueueBase::DestroyImpl() {}
@@ -336,7 +336,7 @@
task->SetFinishedSerial(GetCompletedCommandSerial());
GetDevice()->GetCallbackTaskManager()->AddCallbackTask(std::move(task));
} else {
- mTasksInFlight.Enqueue(std::move(task), serial);
+ mTasksInFlight->Enqueue(std::move(task), serial);
}
}
@@ -346,7 +346,7 @@
}
void QueueBase::TrackPendingTask(std::unique_ptr<TrackTaskCallback> task) {
- mTasksInFlight.Enqueue(std::move(task), GetPendingCommandSerial());
+ mTasksInFlight->Enqueue(std::move(task), GetPendingCommandSerial());
}
void QueueBase::Tick(ExecutionSerial finishedSerial) {
@@ -359,11 +359,12 @@
uint64_t(finishedSerial));
std::vector<std::unique_ptr<TrackTaskCallback>> tasks;
- for (auto& task : mTasksInFlight.IterateUpTo(finishedSerial)) {
- tasks.push_back(std::move(task));
- }
- mTasksInFlight.ClearUpTo(finishedSerial);
-
+ mTasksInFlight.Use([&](auto tasksInFlight) {
+ for (auto& task : tasksInFlight->IterateUpTo(finishedSerial)) {
+ tasks.push_back(std::move(task));
+ }
+ tasksInFlight->ClearUpTo(finishedSerial);
+ });
// Tasks' serials have passed. Move them to the callback task manager. They
// are ready to be called.
for (auto& task : tasks) {
@@ -373,11 +374,13 @@
}
void QueueBase::HandleDeviceLoss() {
- for (auto& task : mTasksInFlight.IterateAll()) {
- task->OnDeviceLoss();
- GetDevice()->GetCallbackTaskManager()->AddCallbackTask(std::move(task));
- }
- mTasksInFlight.Clear();
+ mTasksInFlight.Use([&](auto tasksInFlight) {
+ for (auto& task : tasksInFlight->IterateAll()) {
+ task->OnDeviceLoss();
+ GetDevice()->GetCallbackTaskManager()->AddCallbackTask(std::move(task));
+ }
+ tasksInFlight->Clear();
+ });
}
void QueueBase::APIWriteBuffer(BufferBase* buffer,
diff --git a/src/dawn/native/Queue.h b/src/dawn/native/Queue.h
index ce473f8..6fd2260 100644
--- a/src/dawn/native/Queue.h
+++ b/src/dawn/native/Queue.h
@@ -17,6 +17,7 @@
#include <memory>
+#include "dawn/common/MutexProtected.h"
#include "dawn/common/SerialMap.h"
#include "dawn/native/CallbackTaskManager.h"
#include "dawn/native/Error.h"
@@ -137,7 +138,7 @@
MaybeError SubmitInternal(uint32_t commandCount, CommandBufferBase* const* commands);
- SerialMap<ExecutionSerial, std::unique_ptr<TrackTaskCallback>> mTasksInFlight;
+ MutexProtected<SerialMap<ExecutionSerial, std::unique_ptr<TrackTaskCallback>>> mTasksInFlight;
};
} // namespace dawn::native
diff --git a/src/dawn/native/SwapChain.h b/src/dawn/native/SwapChain.h
index 362e002..fd2a1b1 100644
--- a/src/dawn/native/SwapChain.h
+++ b/src/dawn/native/SwapChain.h
@@ -63,6 +63,14 @@
TextureViewBase* APIGetCurrentTextureView();
void APIPresent();
+ // TODO(crbug.com/dawn/831):
+ // APIRelease() can be called without any synchronization guarantees so we need to use a Release
+ // method that will call LockAndDeleteThis() on destruction.
+ // This is because losing the last reference to the SwapChain will detach its surface which
+ // explicitly destroys the current texture. Explicit destruction of textures is not thread safe
+ // yet.
+ void APIRelease() { ReleaseAndLockBeforeDestroy(); }
+
uint32_t GetWidth() const;
uint32_t GetHeight() const;
wgpu::TextureFormat GetFormat() const;
diff --git a/src/dawn/native/Texture.cpp b/src/dawn/native/Texture.cpp
index 5972cef..22743de 100644
--- a/src/dawn/native/Texture.cpp
+++ b/src/dawn/native/Texture.cpp
@@ -713,6 +713,16 @@
mFormatEnumForReflection(descriptor->format) {}
void TextureBase::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the texture is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the texture.
+ // - Losing the last reference to a swap chain will also call APIDestroy on its
+ // current texture. This is protected by acquiring the global device lock on
+ // the last release. That lock can be removed when APIDestroy is made thread-safe.
+ // - It may be called when the last ref to the texture is dropped and the texture
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the texture since there are no other live refs.
mState.destroyed = true;
// Destroy all of the views associated with the texture as well.
@@ -1158,8 +1168,14 @@
}
ApiObjectList* TextureViewBase::GetObjectTrackingList() {
- DAWN_ASSERT(!IsError());
- return mTexture->GetViewTrackingList();
+ if (mTexture != nullptr) {
+ return mTexture->GetViewTrackingList();
+ }
+ // Return the base device list for error objects so that
+ // the list is never null. Error texture views are never tracked,
+ // so liveness checks will always return false.
+ DAWN_ASSERT(IsError());
+ return ApiObjectBase::GetObjectTrackingList();
}
} // namespace dawn::native
diff --git a/src/dawn/native/d3d/DeviceD3D.cpp b/src/dawn/native/d3d/DeviceD3D.cpp
index 3f80955..dddc36f 100644
--- a/src/dawn/native/d3d/DeviceD3D.cpp
+++ b/src/dawn/native/d3d/DeviceD3D.cpp
@@ -39,6 +39,13 @@
}
void Device::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the device is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the device.
+ // - It may be called when the last ref to the device is dropped and the device
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the device since there are no other live refs.
while (!mExternalImageList.empty()) {
d3d::ExternalImageDXGIImpl* externalImage = mExternalImageList.head()->value();
// ExternalImageDXGIImpl::DestroyInternal() calls RemoveFromList().
diff --git a/src/dawn/native/d3d11/BufferD3D11.cpp b/src/dawn/native/d3d11/BufferD3D11.cpp
index a58b322..0f34450 100644
--- a/src/dawn/native/d3d11/BufferD3D11.cpp
+++ b/src/dawn/native/d3d11/BufferD3D11.cpp
@@ -287,6 +287,13 @@
}
void Buffer::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the buffer is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the buffer.
+ // - It may be called when the last ref to the buffer is dropped and the buffer
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the buffer since there are no other live refs.
BufferBase::DestroyImpl();
if (mMappedData) {
UnmapInternal();
diff --git a/src/dawn/native/d3d11/DeviceD3D11.cpp b/src/dawn/native/d3d11/DeviceD3D11.cpp
index a35922b..47ba460 100644
--- a/src/dawn/native/d3d11/DeviceD3D11.cpp
+++ b/src/dawn/native/d3d11/DeviceD3D11.cpp
@@ -434,6 +434,13 @@
}
void Device::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the device is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the device.
+ // - It may be called when the last ref to the device is dropped and the device
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the device since there are no other live refs.
DAWN_ASSERT(GetState() == State::Disconnected);
Base::DestroyImpl();
diff --git a/src/dawn/native/d3d11/QuerySetD3D11.cpp b/src/dawn/native/d3d11/QuerySetD3D11.cpp
index 107da2e..986055b 100644
--- a/src/dawn/native/d3d11/QuerySetD3D11.cpp
+++ b/src/dawn/native/d3d11/QuerySetD3D11.cpp
@@ -48,6 +48,13 @@
}
void QuerySet::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the query set is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the query set.
+ // - It may be called when the last ref to the query set is dropped and it
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the query set since there are no other live refs.
QuerySetBase::DestroyImpl();
mPredicates.clear();
}
diff --git a/src/dawn/native/d3d11/TextureD3D11.cpp b/src/dawn/native/d3d11/TextureD3D11.cpp
index b56b0f4..a7f74fe 100644
--- a/src/dawn/native/d3d11/TextureD3D11.cpp
+++ b/src/dawn/native/d3d11/TextureD3D11.cpp
@@ -365,6 +365,13 @@
Texture::~Texture() = default;
void Texture::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the texture is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the texture.
+ // - It may be called when the last ref to the texture is dropped and the texture
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the texture since there are no other live refs.
TextureBase::DestroyImpl();
mD3d11Resource = nullptr;
}
diff --git a/src/dawn/native/d3d12/BufferD3D12.cpp b/src/dawn/native/d3d12/BufferD3D12.cpp
index 0985c6a..e082948 100644
--- a/src/dawn/native/d3d12/BufferD3D12.cpp
+++ b/src/dawn/native/d3d12/BufferD3D12.cpp
@@ -459,6 +459,13 @@
}
void Buffer::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the buffer is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the buffer.
+ // - It may be called when the last ref to the buffer is dropped and the buffer
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the buffer since there are no other live refs.
if (mMappedData != nullptr) {
// If the buffer is currently mapped, unmap without flushing the writes to the GPU
// since the buffer cannot be used anymore. UnmapImpl checks mWrittenRange to know
diff --git a/src/dawn/native/d3d12/DeviceD3D12.cpp b/src/dawn/native/d3d12/DeviceD3D12.cpp
index 7da08d2..781c0f9 100644
--- a/src/dawn/native/d3d12/DeviceD3D12.cpp
+++ b/src/dawn/native/d3d12/DeviceD3D12.cpp
@@ -141,8 +141,8 @@
mSamplerHeapCache = std::make_unique<SamplerHeapCache>(this);
- mResidencyManager = std::make_unique<ResidencyManager>(this);
- mResourceAllocatorManager = std::make_unique<ResourceAllocatorManager>(this);
+ mResidencyManager = std::make_unique<MutexProtected<ResidencyManager>>(this);
+ mResourceAllocatorManager = std::make_unique<MutexProtected<ResourceAllocatorManager>>(this);
// ShaderVisibleDescriptorAllocators use the ResidencyManager and must be initialized after.
DAWN_TRY_ASSIGN(
@@ -243,8 +243,8 @@
return mCommandAllocatorManager.get();
}
-ResidencyManager* Device::GetResidencyManager() const {
- return mResidencyManager.get();
+MutexProtected<ResidencyManager>& Device::GetResidencyManager() const {
+ return *mResidencyManager;
}
ResultOrError<CommandRecordingContext*> Device::GetPendingCommandContext(
@@ -315,13 +315,13 @@
// Perform cleanup operations to free unused objects
ExecutionSerial completedSerial = GetQueue()->GetCompletedCommandSerial();
- mResourceAllocatorManager->Tick(completedSerial);
+ (*mResourceAllocatorManager)->Tick(completedSerial);
DAWN_TRY(mCommandAllocatorManager->Tick(completedSerial));
(*mViewShaderVisibleDescriptorAllocator)->Tick(completedSerial);
(*mSamplerShaderVisibleDescriptorAllocator)->Tick(completedSerial);
(*mRenderTargetViewAllocator)->Tick(completedSerial);
(*mDepthStencilViewAllocator)->Tick(completedSerial);
- mUsedComObjectRefs.ClearUpTo(completedSerial);
+ mUsedComObjectRefs->ClearUpTo(completedSerial);
if (mPendingCommands.IsOpen() && mPendingCommands.NeedsSubmit()) {
DAWN_TRY(ExecutePendingCommandContext());
@@ -374,7 +374,7 @@
}
void Device::ReferenceUntilUnused(ComPtr<IUnknown> object) {
- mUsedComObjectRefs.Enqueue(object, GetPendingCommandSerial());
+ mUsedComObjectRefs->Enqueue(std::move(object), GetPendingCommandSerial());
}
bool Device::HasPendingCommands() const {
@@ -567,7 +567,7 @@
}
void Device::DeallocateMemory(ResourceHeapAllocation& allocation) {
- mResourceAllocatorManager->DeallocateMemory(allocation);
+ (*mResourceAllocatorManager)->DeallocateMemory(allocation);
}
ResultOrError<ResourceHeapAllocation> Device::AllocateMemory(
@@ -577,9 +577,9 @@
uint32_t formatBytesPerBlock,
bool forceAllocateAsCommittedResource) {
// formatBytesPerBlock is needed only for color non-compressed formats for a workaround.
- return mResourceAllocatorManager->AllocateMemory(heapType, resourceDescriptor, initialUsage,
- formatBytesPerBlock,
- forceAllocateAsCommittedResource);
+ return (*mResourceAllocatorManager)
+ ->AllocateMemory(heapType, resourceDescriptor, initialUsage, formatBytesPerBlock,
+ forceAllocateAsCommittedResource);
}
ResultOrError<Ref<d3d::Fence>> Device::CreateFence(
@@ -744,6 +744,14 @@
void Device::DestroyImpl() {
DAWN_ASSERT(GetState() == State::Disconnected);
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the device is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the device.
+ // - It may be called when the last ref to the device is dropped and the device
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the device since there are no other live refs.
+
Base::DestroyImpl();
mZeroBuffer = nullptr;
@@ -761,9 +769,9 @@
mResourceAllocatorManager.reset();
// We need to handle clearing up com object refs that were enqeued after TickImpl
- mUsedComObjectRefs.ClearUpTo(std::numeric_limits<ExecutionSerial>::max());
+ mUsedComObjectRefs->ClearUpTo(std::numeric_limits<ExecutionSerial>::max());
- DAWN_ASSERT(mUsedComObjectRefs.Empty());
+ DAWN_ASSERT(mUsedComObjectRefs->Empty());
DAWN_ASSERT(!mPendingCommands.IsOpen());
// Now that we've cleared out pending work from the queue, we can safely release it and reclaim
diff --git a/src/dawn/native/d3d12/DeviceD3D12.h b/src/dawn/native/d3d12/DeviceD3D12.h
index 2072ad5..c10ff15 100644
--- a/src/dawn/native/d3d12/DeviceD3D12.h
+++ b/src/dawn/native/d3d12/DeviceD3D12.h
@@ -71,7 +71,7 @@
ComPtr<ID3D12CommandSignature> GetDrawIndexedIndirectSignature() const;
CommandAllocatorManager* GetCommandAllocatorManager() const;
- ResidencyManager* GetResidencyManager() const;
+ MutexProtected<ResidencyManager>& GetResidencyManager() const;
const PlatformFunctions* GetFunctions() const;
@@ -243,11 +243,11 @@
CommandRecordingContext mPendingCommands;
- SerialQueue<ExecutionSerial, ComPtr<IUnknown>> mUsedComObjectRefs;
+ MutexProtected<SerialQueue<ExecutionSerial, ComPtr<IUnknown>>> mUsedComObjectRefs;
std::unique_ptr<CommandAllocatorManager> mCommandAllocatorManager;
- std::unique_ptr<ResourceAllocatorManager> mResourceAllocatorManager;
- std::unique_ptr<ResidencyManager> mResidencyManager;
+ std::unique_ptr<MutexProtected<ResourceAllocatorManager>> mResourceAllocatorManager;
+ std::unique_ptr<MutexProtected<ResidencyManager>> mResidencyManager;
static constexpr uint32_t kMaxSamplerDescriptorsPerBindGroup = 3 * kMaxSamplersPerShaderStage;
static constexpr uint32_t kMaxViewDescriptorsPerBindGroup =
diff --git a/src/dawn/native/d3d12/QuerySetD3D12.cpp b/src/dawn/native/d3d12/QuerySetD3D12.cpp
index 9f7ed07..893d345 100644
--- a/src/dawn/native/d3d12/QuerySetD3D12.cpp
+++ b/src/dawn/native/d3d12/QuerySetD3D12.cpp
@@ -65,6 +65,13 @@
QuerySet::~QuerySet() = default;
void QuerySet::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the query set is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the query set.
+ // - It may be called when the last ref to the query set is dropped and it
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the query set since there are no other live refs.
QuerySetBase::DestroyImpl();
ToBackend(GetDevice())->ReferenceUntilUnused(mQueryHeap);
mQueryHeap = nullptr;
diff --git a/src/dawn/native/metal/BufferMTL.mm b/src/dawn/native/metal/BufferMTL.mm
index ae49bca..c7dbcac 100644
--- a/src/dawn/native/metal/BufferMTL.mm
+++ b/src/dawn/native/metal/BufferMTL.mm
@@ -199,6 +199,13 @@
}
void Buffer::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the buffer is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the buffer.
+ // - It may be called when the last ref to the buffer is dropped and the buffer
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the buffer since there are no other live refs.
BufferBase::DestroyImpl();
mMtlBuffer = nullptr;
}
diff --git a/src/dawn/native/metal/DeviceMTL.mm b/src/dawn/native/metal/DeviceMTL.mm
index 0aa58e2..519a277 100644
--- a/src/dawn/native/metal/DeviceMTL.mm
+++ b/src/dawn/native/metal/DeviceMTL.mm
@@ -379,7 +379,13 @@
void Device::DestroyImpl() {
DAWN_ASSERT(GetState() == State::Disconnected);
-
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the device is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the device.
+ // - It may be called when the last ref to the device is dropped and the device
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the device since there are no other live refs.
mMtlDevice = nullptr;
mMockBlitMtlBuffer = nullptr;
}
diff --git a/src/dawn/native/metal/QuerySetMTL.mm b/src/dawn/native/metal/QuerySetMTL.mm
index 4a5a24c..7b58ed8 100644
--- a/src/dawn/native/metal/QuerySetMTL.mm
+++ b/src/dawn/native/metal/QuerySetMTL.mm
@@ -136,6 +136,13 @@
QuerySet::~QuerySet() = default;
void QuerySet::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the query set is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the query set.
+ // - It may be called when the last ref to the query set is dropped andit
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the query set since there are no other live refs.
QuerySetBase::DestroyImpl();
mVisibilityBuffer = nullptr;
diff --git a/src/dawn/native/metal/TextureMTL.mm b/src/dawn/native/metal/TextureMTL.mm
index 18be29e..a26037c 100644
--- a/src/dawn/native/metal/TextureMTL.mm
+++ b/src/dawn/native/metal/TextureMTL.mm
@@ -892,6 +892,13 @@
Texture::~Texture() {}
void Texture::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the texture is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the texture.
+ // - It may be called when the last ref to the texture is dropped and the texture
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the texture since there are no other live refs.
TextureBase::DestroyImpl();
mMtlTexture = nullptr;
mIOSurface = nullptr;
diff --git a/src/dawn/native/null/DeviceNull.cpp b/src/dawn/native/null/DeviceNull.cpp
index 25891c2..998bcf5 100644
--- a/src/dawn/native/null/DeviceNull.cpp
+++ b/src/dawn/native/null/DeviceNull.cpp
@@ -210,6 +210,13 @@
void Device::DestroyImpl() {
DAWN_ASSERT(GetState() == State::Disconnected);
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the device is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the device.
+ // - It may be called when the last ref to the device is dropped and the device
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the device since there are no other live refs.
// Clear pending operations before checking mMemoryUsage because some operations keep a
// reference to Buffers.
@@ -348,6 +355,13 @@
void Buffer::UnmapImpl() {}
void Buffer::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the buffer is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the buffer.
+ // - It may be called when the last ref to the buffer is dropped and the buffer
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the buffer since there are no other live refs.
BufferBase::DestroyImpl();
ToBackend(GetDevice())->DecrementMemoryUsage(GetSize());
}
diff --git a/src/dawn/native/vulkan/BufferVk.cpp b/src/dawn/native/vulkan/BufferVk.cpp
index 4744cf3..ad3196e 100644
--- a/src/dawn/native/vulkan/BufferVk.cpp
+++ b/src/dawn/native/vulkan/BufferVk.cpp
@@ -471,6 +471,13 @@
}
void Buffer::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the buffer is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the buffer.
+ // - It may be called when the last ref to the buffer is dropped and the buffer
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the buffer since there are no other live refs.
BufferBase::DestroyImpl();
ToBackend(GetDevice())->GetResourceMemoryAllocator()->Deallocate(&mMemoryAllocation);
diff --git a/src/dawn/native/vulkan/DeviceVk.cpp b/src/dawn/native/vulkan/DeviceVk.cpp
index 455add0..99cc1ec 100644
--- a/src/dawn/native/vulkan/DeviceVk.cpp
+++ b/src/dawn/native/vulkan/DeviceVk.cpp
@@ -128,11 +128,11 @@
// the device.
GatherQueueFromDevice();
- mDeleter = std::make_unique<FencedDeleter>(this);
+ mDeleter = std::make_unique<MutexProtected<FencedDeleter>>(this);
}
mRenderPassCache = std::make_unique<RenderPassCache>(this);
- mResourceMemoryAllocator = std::make_unique<ResourceMemoryAllocator>(this);
+ mResourceMemoryAllocator = std::make_unique<MutexProtected<ResourceMemoryAllocator>>(this);
mExternalMemoryService = std::make_unique<external_memory::Service>(this);
mExternalSemaphoreService = std::make_unique<external_semaphore::Service>(this);
@@ -232,8 +232,8 @@
allocator->FinishDeallocation(completedSerial);
}
- mResourceMemoryAllocator->Tick(completedSerial);
- mDeleter->Tick(completedSerial);
+ GetResourceMemoryAllocator()->Tick(completedSerial);
+ GetFencedDeleter()->Tick(completedSerial);
mDescriptorAllocatorsPendingDeallocation.ClearUpTo(completedSerial);
if (mRecordingContext.needsSubmit) {
@@ -268,16 +268,16 @@
return mQueue;
}
-FencedDeleter* Device::GetFencedDeleter() const {
- return mDeleter.get();
+MutexProtected<FencedDeleter>& Device::GetFencedDeleter() const {
+ return *mDeleter;
}
RenderPassCache* Device::GetRenderPassCache() const {
return mRenderPassCache.get();
}
-ResourceMemoryAllocator* Device::GetResourceMemoryAllocator() const {
- return mResourceMemoryAllocator.get();
+MutexProtected<ResourceMemoryAllocator>& Device::GetResourceMemoryAllocator() const {
+ return *mResourceMemoryAllocator;
}
external_semaphore::Service* Device::GetExternalSemaphoreService() const {
@@ -366,7 +366,7 @@
// Enqueue the semaphores before incrementing the serial, so that they can be deleted as
// soon as the current submission is finished.
for (VkSemaphore semaphore : mRecordingContext.waitSemaphores) {
- mDeleter->DeleteWhenUnused(semaphore);
+ GetFencedDeleter()->DeleteWhenUnused(semaphore);
}
GetQueue()->IncrementLastSubmittedCommandSerial();
ExecutionSerial lastSubmittedSerial = GetLastSubmittedCommandSerial();
@@ -1054,6 +1054,14 @@
return;
}
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the device is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the device.
+ // - It may be called when the last ref to the device is dropped and the device
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the device since there are no other live refs.
+
// The deleter is the second thing we initialize. If it is not present, it means that
// only the VkDevice was created and nothing else. Destroy the device and do nothing else
// because the function pointers might not have been loaded (and there is nothing to
@@ -1111,11 +1119,11 @@
// Releasing the uploader enqueues buffers to be released.
// Call Tick() again to clear them before releasing the deleter.
- mResourceMemoryAllocator->Tick(kMaxExecutionSerial);
+ GetResourceMemoryAllocator()->Tick(kMaxExecutionSerial);
mDescriptorAllocatorsPendingDeallocation.ClearUpTo(kMaxExecutionSerial);
// Allow recycled memory to be deleted.
- mResourceMemoryAllocator->DestroyPool();
+ GetResourceMemoryAllocator()->DestroyPool();
// The VkRenderPasses in the cache can be destroyed immediately since all commands referring
// to them are guaranteed to be finished executing.
@@ -1123,7 +1131,7 @@
// Delete all the remaining VkDevice child objects immediately since the GPU timeline is
// finished.
- mDeleter->Tick(kMaxExecutionSerial);
+ GetFencedDeleter()->Tick(kMaxExecutionSerial);
mDeleter = nullptr;
// VkQueues are destroyed when the VkDevice is destroyed
diff --git a/src/dawn/native/vulkan/DeviceVk.h b/src/dawn/native/vulkan/DeviceVk.h
index 9376d76..ef842d6 100644
--- a/src/dawn/native/vulkan/DeviceVk.h
+++ b/src/dawn/native/vulkan/DeviceVk.h
@@ -21,6 +21,7 @@
#include <utility>
#include <vector>
+#include "dawn/common/MutexProtected.h"
#include "dawn/common/SerialQueue.h"
#include "dawn/native/Commands.h"
#include "dawn/native/Device.h"
@@ -60,9 +61,9 @@
uint32_t GetGraphicsQueueFamily() const;
VkQueue GetVkQueue() const;
- FencedDeleter* GetFencedDeleter() const;
+ MutexProtected<FencedDeleter>& GetFencedDeleter() const;
RenderPassCache* GetRenderPassCache() const;
- ResourceMemoryAllocator* GetResourceMemoryAllocator() const;
+ MutexProtected<ResourceMemoryAllocator>& GetResourceMemoryAllocator() const;
external_semaphore::Service* GetExternalSemaphoreService() const;
CommandRecordingContext* GetPendingRecordingContext(
@@ -183,8 +184,8 @@
SerialQueue<ExecutionSerial, Ref<DescriptorSetAllocator>>
mDescriptorAllocatorsPendingDeallocation;
- std::unique_ptr<FencedDeleter> mDeleter;
- std::unique_ptr<ResourceMemoryAllocator> mResourceMemoryAllocator;
+ std::unique_ptr<MutexProtected<FencedDeleter>> mDeleter;
+ std::unique_ptr<MutexProtected<ResourceMemoryAllocator>> mResourceMemoryAllocator;
std::unique_ptr<RenderPassCache> mRenderPassCache;
std::unique_ptr<external_memory::Service> mExternalMemoryService;
diff --git a/src/dawn/native/vulkan/QuerySetVk.cpp b/src/dawn/native/vulkan/QuerySetVk.cpp
index 36eaec6..338af95 100644
--- a/src/dawn/native/vulkan/QuerySetVk.cpp
+++ b/src/dawn/native/vulkan/QuerySetVk.cpp
@@ -101,6 +101,13 @@
QuerySet::~QuerySet() = default;
void QuerySet::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the query set is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the query set.
+ // - It may be called when the last ref to the query set is dropped and it
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the query set since there are no other live refs.
QuerySetBase::DestroyImpl();
if (mHandle != VK_NULL_HANDLE) {
ToBackend(GetDevice())->GetFencedDeleter()->DeleteWhenUnused(mHandle);
diff --git a/src/dawn/native/vulkan/TextureVk.cpp b/src/dawn/native/vulkan/TextureVk.cpp
index d71df30..ecb7493 100644
--- a/src/dawn/native/vulkan/TextureVk.cpp
+++ b/src/dawn/native/vulkan/TextureVk.cpp
@@ -1020,6 +1020,13 @@
}
void Texture::DestroyImpl() {
+ // TODO(crbug.com/dawn/831): DestroyImpl is called from two places.
+ // - It may be called if the texture is explicitly destroyed with APIDestroy.
+ // This case is NOT thread-safe and needs proper synchronization with other
+ // simultaneous uses of the texture.
+ // - It may be called when the last ref to the texture is dropped and the texture
+ // is implicitly destroyed. This case is thread-safe because there are no
+ // other threads using the texture since there are no other live refs.
Device* device = ToBackend(GetDevice());
if (mOwnsHandle) {
