Don't leak buffers if MapAtCreation fails.

This required changing DeviceBase::CreateBufferImpl to return
ResultOrError<Ref<BufferBase>>

Bug: chromium:1103154
Change-Id: I1a5811d293333b6ef29c988a08f2f1f84ac65702
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/24500
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
This commit is contained in:
Corentin Wallez 2020-07-08 19:45:40 +00:00 committed by Commit Bot service account
parent 519edd5890
commit 8a9919980f
17 changed files with 36 additions and 28 deletions

View File

@ -75,4 +75,4 @@ void RefCounted::Release() {
void RefCounted::DeleteThis() { void RefCounted::DeleteThis() {
delete this; delete this;
} }

View File

@ -604,13 +604,13 @@ namespace dawn_native {
return result; return result;
} }
BufferBase* DeviceBase::CreateBuffer(const BufferDescriptor* descriptor) { BufferBase* DeviceBase::CreateBuffer(const BufferDescriptor* descriptor) {
BufferBase* result = nullptr; Ref<BufferBase> result = nullptr;
if (ConsumedError(CreateBufferInternal(descriptor), &result)) { if (ConsumedError(CreateBufferInternal(descriptor), &result)) {
ASSERT(result == nullptr); ASSERT(result.Get() == nullptr);
return BufferBase::MakeError(this, descriptor); return BufferBase::MakeError(this, descriptor);
} }
return result; return result.Detach();
} }
WGPUCreateBufferMappedResult DeviceBase::CreateBufferMapped( WGPUCreateBufferMappedResult DeviceBase::CreateBufferMapped(
const BufferDescriptor* descriptor) { const BufferDescriptor* descriptor) {
@ -870,21 +870,21 @@ namespace dawn_native {
return {}; return {};
} }
ResultOrError<BufferBase*> DeviceBase::CreateBufferInternal( ResultOrError<Ref<BufferBase>> DeviceBase::CreateBufferInternal(
const BufferDescriptor* descriptor) { const BufferDescriptor* descriptor) {
DAWN_TRY(ValidateIsAlive()); DAWN_TRY(ValidateIsAlive());
if (IsValidationEnabled()) { if (IsValidationEnabled()) {
DAWN_TRY(ValidateBufferDescriptor(this, descriptor)); DAWN_TRY(ValidateBufferDescriptor(this, descriptor));
} }
BufferBase* buffer = nullptr; Ref<BufferBase> buffer;
DAWN_TRY_ASSIGN(buffer, CreateBufferImpl(descriptor)); DAWN_TRY_ASSIGN(buffer, CreateBufferImpl(descriptor));
if (descriptor->mappedAtCreation) { if (descriptor->mappedAtCreation) {
DAWN_TRY(buffer->MapAtCreation()); DAWN_TRY(buffer->MapAtCreation());
} }
return buffer; return std::move(buffer);
} }
MaybeError DeviceBase::CreateComputePipelineInternal( MaybeError DeviceBase::CreateComputePipelineInternal(

View File

@ -241,7 +241,8 @@ namespace dawn_native {
const BindGroupDescriptor* descriptor) = 0; const BindGroupDescriptor* descriptor) = 0;
virtual ResultOrError<BindGroupLayoutBase*> CreateBindGroupLayoutImpl( virtual ResultOrError<BindGroupLayoutBase*> CreateBindGroupLayoutImpl(
const BindGroupLayoutDescriptor* descriptor) = 0; const BindGroupLayoutDescriptor* descriptor) = 0;
virtual ResultOrError<BufferBase*> CreateBufferImpl(const BufferDescriptor* descriptor) = 0; virtual ResultOrError<Ref<BufferBase>> CreateBufferImpl(
const BufferDescriptor* descriptor) = 0;
virtual ResultOrError<ComputePipelineBase*> CreateComputePipelineImpl( virtual ResultOrError<ComputePipelineBase*> CreateComputePipelineImpl(
const ComputePipelineDescriptor* descriptor) = 0; const ComputePipelineDescriptor* descriptor) = 0;
virtual ResultOrError<PipelineLayoutBase*> CreatePipelineLayoutImpl( virtual ResultOrError<PipelineLayoutBase*> CreatePipelineLayoutImpl(
@ -273,7 +274,7 @@ namespace dawn_native {
const BindGroupDescriptor* descriptor); const BindGroupDescriptor* descriptor);
MaybeError CreateBindGroupLayoutInternal(BindGroupLayoutBase** result, MaybeError CreateBindGroupLayoutInternal(BindGroupLayoutBase** result,
const BindGroupLayoutDescriptor* descriptor); const BindGroupLayoutDescriptor* descriptor);
ResultOrError<BufferBase*> CreateBufferInternal(const BufferDescriptor* descriptor); ResultOrError<Ref<BufferBase>> CreateBufferInternal(const BufferDescriptor* descriptor);
MaybeError CreateComputePipelineInternal(ComputePipelineBase** result, MaybeError CreateComputePipelineInternal(ComputePipelineBase** result,
const ComputePipelineDescriptor* descriptor); const ComputePipelineDescriptor* descriptor);
MaybeError CreatePipelineLayoutInternal(PipelineLayoutBase** result, MaybeError CreatePipelineLayoutInternal(PipelineLayoutBase** result,

View File

@ -270,10 +270,10 @@ namespace dawn_native { namespace d3d12 {
const BindGroupLayoutDescriptor* descriptor) { const BindGroupLayoutDescriptor* descriptor) {
return new BindGroupLayout(this, descriptor); return new BindGroupLayout(this, descriptor);
} }
ResultOrError<BufferBase*> Device::CreateBufferImpl(const BufferDescriptor* descriptor) { ResultOrError<Ref<BufferBase>> Device::CreateBufferImpl(const BufferDescriptor* descriptor) {
Ref<Buffer> buffer = AcquireRef(new Buffer(this, descriptor)); Ref<Buffer> buffer = AcquireRef(new Buffer(this, descriptor));
DAWN_TRY(buffer->Initialize()); DAWN_TRY(buffer->Initialize());
return buffer.Detach(); return std::move(buffer);
} }
CommandBufferBase* Device::CreateCommandBuffer(CommandEncoder* encoder, CommandBufferBase* Device::CreateCommandBuffer(CommandEncoder* encoder,
const CommandBufferDescriptor* descriptor) { const CommandBufferDescriptor* descriptor) {

View File

@ -138,7 +138,8 @@ namespace dawn_native { namespace d3d12 {
const BindGroupDescriptor* descriptor) override; const BindGroupDescriptor* descriptor) override;
ResultOrError<BindGroupLayoutBase*> CreateBindGroupLayoutImpl( ResultOrError<BindGroupLayoutBase*> CreateBindGroupLayoutImpl(
const BindGroupLayoutDescriptor* descriptor) override; const BindGroupLayoutDescriptor* descriptor) override;
ResultOrError<BufferBase*> CreateBufferImpl(const BufferDescriptor* descriptor) override; ResultOrError<Ref<BufferBase>> CreateBufferImpl(
const BufferDescriptor* descriptor) override;
ResultOrError<ComputePipelineBase*> CreateComputePipelineImpl( ResultOrError<ComputePipelineBase*> CreateComputePipelineImpl(
const ComputePipelineDescriptor* descriptor) override; const ComputePipelineDescriptor* descriptor) override;
ResultOrError<PipelineLayoutBase*> CreatePipelineLayoutImpl( ResultOrError<PipelineLayoutBase*> CreatePipelineLayoutImpl(

View File

@ -27,7 +27,8 @@ namespace dawn_native { namespace metal {
class Buffer : public BufferBase { class Buffer : public BufferBase {
public: public:
static ResultOrError<Buffer*> Create(Device* device, const BufferDescriptor* descriptor); static ResultOrError<Ref<Buffer>> Create(Device* device,
const BufferDescriptor* descriptor);
id<MTLBuffer> GetMTLBuffer() const; id<MTLBuffer> GetMTLBuffer() const;
void ClearBufferContentsToZero(CommandRecordingContext* commandContext); void ClearBufferContentsToZero(CommandRecordingContext* commandContext);

View File

@ -30,10 +30,10 @@ namespace dawn_native { namespace metal {
static constexpr uint32_t kMaxBufferSizeFallback = 1024u * 1024u * 1024u; static constexpr uint32_t kMaxBufferSizeFallback = 1024u * 1024u * 1024u;
// static // static
ResultOrError<Buffer*> Buffer::Create(Device* device, const BufferDescriptor* descriptor) { ResultOrError<Ref<Buffer>> Buffer::Create(Device* device, const BufferDescriptor* descriptor) {
Ref<Buffer> buffer = AcquireRef(new Buffer(device, descriptor)); Ref<Buffer> buffer = AcquireRef(new Buffer(device, descriptor));
DAWN_TRY(buffer->Initialize()); DAWN_TRY(buffer->Initialize());
return buffer.Detach(); return std::move(buffer);
} }
MaybeError Buffer::Initialize() { MaybeError Buffer::Initialize() {

View File

@ -71,7 +71,8 @@ namespace dawn_native { namespace metal {
const BindGroupDescriptor* descriptor) override; const BindGroupDescriptor* descriptor) override;
ResultOrError<BindGroupLayoutBase*> CreateBindGroupLayoutImpl( ResultOrError<BindGroupLayoutBase*> CreateBindGroupLayoutImpl(
const BindGroupLayoutDescriptor* descriptor) override; const BindGroupLayoutDescriptor* descriptor) override;
ResultOrError<BufferBase*> CreateBufferImpl(const BufferDescriptor* descriptor) override; ResultOrError<Ref<BufferBase>> CreateBufferImpl(
const BufferDescriptor* descriptor) override;
ResultOrError<ComputePipelineBase*> CreateComputePipelineImpl( ResultOrError<ComputePipelineBase*> CreateComputePipelineImpl(
const ComputePipelineDescriptor* descriptor) override; const ComputePipelineDescriptor* descriptor) override;
ResultOrError<PipelineLayoutBase*> CreatePipelineLayoutImpl( ResultOrError<PipelineLayoutBase*> CreatePipelineLayoutImpl(

View File

@ -108,7 +108,7 @@ namespace dawn_native { namespace metal {
const BindGroupLayoutDescriptor* descriptor) { const BindGroupLayoutDescriptor* descriptor) {
return new BindGroupLayout(this, descriptor); return new BindGroupLayout(this, descriptor);
} }
ResultOrError<BufferBase*> Device::CreateBufferImpl(const BufferDescriptor* descriptor) { ResultOrError<Ref<BufferBase>> Device::CreateBufferImpl(const BufferDescriptor* descriptor) {
return Buffer::Create(this, descriptor); return Buffer::Create(this, descriptor);
} }
CommandBufferBase* Device::CreateCommandBuffer(CommandEncoder* encoder, CommandBufferBase* Device::CreateCommandBuffer(CommandEncoder* encoder,

View File

@ -100,9 +100,9 @@ namespace dawn_native { namespace null {
const BindGroupLayoutDescriptor* descriptor) { const BindGroupLayoutDescriptor* descriptor) {
return new BindGroupLayout(this, descriptor); return new BindGroupLayout(this, descriptor);
} }
ResultOrError<BufferBase*> Device::CreateBufferImpl(const BufferDescriptor* descriptor) { ResultOrError<Ref<BufferBase>> Device::CreateBufferImpl(const BufferDescriptor* descriptor) {
DAWN_TRY(IncrementMemoryUsage(descriptor->size)); DAWN_TRY(IncrementMemoryUsage(descriptor->size));
return new Buffer(this, descriptor); return AcquireRef(new Buffer(this, descriptor));
} }
CommandBufferBase* Device::CreateCommandBuffer(CommandEncoder* encoder, CommandBufferBase* Device::CreateCommandBuffer(CommandEncoder* encoder,
const CommandBufferDescriptor* descriptor) { const CommandBufferDescriptor* descriptor) {

View File

@ -116,7 +116,8 @@ namespace dawn_native { namespace null {
const BindGroupDescriptor* descriptor) override; const BindGroupDescriptor* descriptor) override;
ResultOrError<BindGroupLayoutBase*> CreateBindGroupLayoutImpl( ResultOrError<BindGroupLayoutBase*> CreateBindGroupLayoutImpl(
const BindGroupLayoutDescriptor* descriptor) override; const BindGroupLayoutDescriptor* descriptor) override;
ResultOrError<BufferBase*> CreateBufferImpl(const BufferDescriptor* descriptor) override; ResultOrError<Ref<BufferBase>> CreateBufferImpl(
const BufferDescriptor* descriptor) override;
ResultOrError<ComputePipelineBase*> CreateComputePipelineImpl( ResultOrError<ComputePipelineBase*> CreateComputePipelineImpl(
const ComputePipelineDescriptor* descriptor) override; const ComputePipelineDescriptor* descriptor) override;
ResultOrError<PipelineLayoutBase*> CreatePipelineLayoutImpl( ResultOrError<PipelineLayoutBase*> CreatePipelineLayoutImpl(

View File

@ -102,8 +102,8 @@ namespace dawn_native { namespace opengl {
const BindGroupLayoutDescriptor* descriptor) { const BindGroupLayoutDescriptor* descriptor) {
return new BindGroupLayout(this, descriptor); return new BindGroupLayout(this, descriptor);
} }
ResultOrError<BufferBase*> Device::CreateBufferImpl(const BufferDescriptor* descriptor) { ResultOrError<Ref<BufferBase>> Device::CreateBufferImpl(const BufferDescriptor* descriptor) {
return new Buffer(this, descriptor); return AcquireRef(new Buffer(this, descriptor));
} }
CommandBufferBase* Device::CreateCommandBuffer(CommandEncoder* encoder, CommandBufferBase* Device::CreateCommandBuffer(CommandEncoder* encoder,
const CommandBufferDescriptor* descriptor) { const CommandBufferDescriptor* descriptor) {

View File

@ -71,7 +71,8 @@ namespace dawn_native { namespace opengl {
const BindGroupDescriptor* descriptor) override; const BindGroupDescriptor* descriptor) override;
ResultOrError<BindGroupLayoutBase*> CreateBindGroupLayoutImpl( ResultOrError<BindGroupLayoutBase*> CreateBindGroupLayoutImpl(
const BindGroupLayoutDescriptor* descriptor) override; const BindGroupLayoutDescriptor* descriptor) override;
ResultOrError<BufferBase*> CreateBufferImpl(const BufferDescriptor* descriptor) override; ResultOrError<Ref<BufferBase>> CreateBufferImpl(
const BufferDescriptor* descriptor) override;
ResultOrError<ComputePipelineBase*> CreateComputePipelineImpl( ResultOrError<ComputePipelineBase*> CreateComputePipelineImpl(
const ComputePipelineDescriptor* descriptor) override; const ComputePipelineDescriptor* descriptor) override;
ResultOrError<PipelineLayoutBase*> CreatePipelineLayoutImpl( ResultOrError<PipelineLayoutBase*> CreatePipelineLayoutImpl(

View File

@ -116,10 +116,10 @@ namespace dawn_native { namespace vulkan {
} // namespace } // namespace
// static // static
ResultOrError<Buffer*> Buffer::Create(Device* device, const BufferDescriptor* descriptor) { ResultOrError<Ref<Buffer>> Buffer::Create(Device* device, const BufferDescriptor* descriptor) {
Ref<Buffer> buffer = AcquireRef(new Buffer(device, descriptor)); Ref<Buffer> buffer = AcquireRef(new Buffer(device, descriptor));
DAWN_TRY(buffer->Initialize()); DAWN_TRY(buffer->Initialize());
return buffer.Detach(); return std::move(buffer);
} }
MaybeError Buffer::Initialize() { MaybeError Buffer::Initialize() {

View File

@ -28,7 +28,8 @@ namespace dawn_native { namespace vulkan {
class Buffer final : public BufferBase { class Buffer final : public BufferBase {
public: public:
static ResultOrError<Buffer*> Create(Device* device, const BufferDescriptor* descriptor); static ResultOrError<Ref<Buffer>> Create(Device* device,
const BufferDescriptor* descriptor);
VkBuffer GetHandle() const; VkBuffer GetHandle() const;

View File

@ -110,7 +110,7 @@ namespace dawn_native { namespace vulkan {
const BindGroupLayoutDescriptor* descriptor) { const BindGroupLayoutDescriptor* descriptor) {
return BindGroupLayout::Create(this, descriptor); return BindGroupLayout::Create(this, descriptor);
} }
ResultOrError<BufferBase*> Device::CreateBufferImpl(const BufferDescriptor* descriptor) { ResultOrError<Ref<BufferBase>> Device::CreateBufferImpl(const BufferDescriptor* descriptor) {
return Buffer::Create(this, descriptor); return Buffer::Create(this, descriptor);
} }
CommandBufferBase* Device::CreateCommandBuffer(CommandEncoder* encoder, CommandBufferBase* Device::CreateCommandBuffer(CommandEncoder* encoder,

View File

@ -107,7 +107,8 @@ namespace dawn_native { namespace vulkan {
const BindGroupDescriptor* descriptor) override; const BindGroupDescriptor* descriptor) override;
ResultOrError<BindGroupLayoutBase*> CreateBindGroupLayoutImpl( ResultOrError<BindGroupLayoutBase*> CreateBindGroupLayoutImpl(
const BindGroupLayoutDescriptor* descriptor) override; const BindGroupLayoutDescriptor* descriptor) override;
ResultOrError<BufferBase*> CreateBufferImpl(const BufferDescriptor* descriptor) override; ResultOrError<Ref<BufferBase>> CreateBufferImpl(
const BufferDescriptor* descriptor) override;
ResultOrError<ComputePipelineBase*> CreateComputePipelineImpl( ResultOrError<ComputePipelineBase*> CreateComputePipelineImpl(
const ComputePipelineDescriptor* descriptor) override; const ComputePipelineDescriptor* descriptor) override;
ResultOrError<PipelineLayoutBase*> CreatePipelineLayoutImpl( ResultOrError<PipelineLayoutBase*> CreatePipelineLayoutImpl(