Fix DynamicUploader use-after-free on ReduceMemoryUsage
Make DynamicUploader work correctly if its list of ring buffer is empty
e.g. after calling ReduceMemoryUsage(). Also, make the ReduceMemoryUsage
test exercise this case.
Bug: 357139493
Change-Id: I87140ca65ad45a276128ba88ba9b738c2c32680b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/202874
Commit-Queue: Austin Eng <enga@chromium.org>
Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/src/dawn/native/DynamicUploader.cpp b/src/dawn/native/DynamicUploader.cpp
index 77930ac..7c3693a 100644
--- a/src/dawn/native/DynamicUploader.cpp
+++ b/src/dawn/native/DynamicUploader.cpp
@@ -36,10 +36,7 @@
namespace dawn::native {
-DynamicUploader::DynamicUploader(DeviceBase* device) : mDevice(device) {
- mRingBuffers.emplace_back(
- std::unique_ptr<RingBuffer>(new RingBuffer{nullptr, RingBufferAllocator(kRingBufferSize)}));
-}
+DynamicUploader::DynamicUploader(DeviceBase* device) : mDevice(device) {}
void DynamicUploader::ReleaseStagingBuffer(Ref<BufferBase> stagingBuffer) {
mReleasedStagingBuffers.Enqueue(std::move(stagingBuffer),
@@ -69,6 +66,11 @@
return uploadHandle;
}
+ if (mRingBuffers.empty()) {
+ mRingBuffers.emplace_back(std::unique_ptr<RingBuffer>(
+ new RingBuffer{nullptr, RingBufferAllocator(kRingBufferSize)}));
+ }
+
// Note: Validation ensures size is already aligned.
// First-fit: find next buffer large enough to satisfy the allocation request.
uint64_t startOffset = RingBufferAllocator::kInvalidOffset;
diff --git a/src/dawn/tests/unittests/native/MemoryInstrumentationTests.cpp b/src/dawn/tests/unittests/native/MemoryInstrumentationTests.cpp
index ca1dfee..cbec419 100644
--- a/src/dawn/tests/unittests/native/MemoryInstrumentationTests.cpp
+++ b/src/dawn/tests/unittests/native/MemoryInstrumentationTests.cpp
@@ -224,19 +224,26 @@
uniformBuffer.Destroy();
- wgpu::Future completionFuture = device.GetQueue().OnSubmittedWorkDone(
- wgpu::CallbackMode::WaitAnyOnly, [](wgpu::QueueWorkDoneStatus status) {});
-
- wgpu::WaitStatus waitStatus = wgpu::WaitStatus::TimedOut;
- while (waitStatus != wgpu::WaitStatus::Success) {
- std::this_thread::sleep_for(std::chrono::milliseconds(100));
- waitStatus = wgpu::Instance(ToAPI(mDeviceMock->GetInstance())).WaitAny(completionFuture, 0);
- }
+ mDeviceMock->GetInstance()->APIProcessEvents();
// DynamicUploader buffers will still be alive.
EXPECT_GT(ComputeEstimatedMemoryUsage(device.Get()), uint64_t(0));
ReduceMemoryUsage(device.Get());
+ // But not any more.
EXPECT_EQ(ComputeEstimatedMemoryUsage(device.Get()), uint64_t(0));
+
+ // Check that DynamicUploader buffer is recreated again.
+ uniformBuffer = device.CreateBuffer(&kBufferDesc);
+ EXPECT_TRUE(uniformBuffer);
+
+ device.GetQueue().WriteBuffer(uniformBuffer, 0, zeroes.data(), zeroes.size());
+ device.GetQueue().Submit(0, nullptr);
+
+ uniformBuffer.Destroy();
+
+ mDeviceMock->GetInstance()->APIProcessEvents();
+
+ EXPECT_GT(ComputeEstimatedMemoryUsage(device.Get()), uint64_t(0));
}
} // namespace
