Commit e29c62b7 authored by Anna Henningsen's avatar Anna Henningsen Committed by Commit Bot

[arraybuffer] Clean up BackingStore even if it pointer to nullptr

For a zero-length BackingStore allocation, it is valid for the
underlying memory to be a null pointer. However, some cleanup
is still necessary, since the BackingStore may hold a reference
to the allocator itself, which needs to be released when destroying
the `BackingStore` instance.

Change-Id: I1f168079d39e4592d2fde31fbe5f705586690e85
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2169646Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67420}
parent 6f4991fa
......@@ -165,7 +165,10 @@ void BackingStore::Clear() {
BackingStore::~BackingStore() {
GlobalBackingStoreRegistry::Unregister(this);
if (buffer_start_ == nullptr) return; // nothing to deallocate
if (buffer_start_ == nullptr) {
Clear();
return;
}
if (is_wasm_memory_) {
DCHECK(free_on_destruct_);
......
......@@ -799,6 +799,46 @@ TEST(BackingStore_HoldAllocatorAlive_AfterIsolateShutdown) {
CHECK(allocator_weak.expired());
}
class NullptrAllocator final : public v8::ArrayBuffer::Allocator {
public:
void* Allocate(size_t length) override {
CHECK_EQ(length, 0);
return nullptr;
}
void* AllocateUninitialized(size_t length) override {
CHECK_EQ(length, 0);
return nullptr;
}
void Free(void* data, size_t length) override { CHECK_EQ(data, nullptr); }
};
TEST(BackingStore_ReleaseAllocator_NullptrBackingStore) {
std::shared_ptr<NullptrAllocator> allocator =
std::make_shared<NullptrAllocator>();
std::weak_ptr<NullptrAllocator> allocator_weak(allocator);
v8::Isolate::CreateParams create_params;
create_params.array_buffer_allocator_shared = allocator;
v8::Isolate* isolate = v8::Isolate::New(create_params);
isolate->Enter();
allocator.reset();
create_params.array_buffer_allocator_shared.reset();
CHECK(!allocator_weak.expired());
{
std::shared_ptr<v8::BackingStore> backing_store =
v8::ArrayBuffer::NewBackingStore(isolate, 0);
// This should release a reference to the allocator, even though the
// buffer is empty/nullptr.
backing_store.reset();
}
isolate->Exit();
isolate->Dispose();
CHECK(allocator_weak.expired());
}
TEST(BackingStore_ReallocateExpand) {
LocalContext env;
v8::Isolate* isolate = env->GetIsolate();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment