Skip to content

Commit fcdf35e

Browse files
ulanCommit Bot
ulan
authored and
Commit Bot
committed
Skip global registration of [Shared]ArrayBuffer backing stores
Previously we needed to register the backing stores globally because the embedder could create them from a raw pointer. This is no longer possible after the removal of the old API. The global backing store registry now keeps track only of wasm memory backing stores. Bug: v8:9380 Change-Id: Iffefbf14dcafc1f9ce0dc3613335c754c9cb649a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2763874 Reviewed-by: Andreas Haas <[email protected]> Commit-Queue: Ulan Degenbaev <[email protected]> Cr-Commit-Position: refs/heads/master@{#73493}
1 parent 63661ce commit fcdf35e

File tree

3 files changed

+8
-42
lines changed

3 files changed

+8
-42
lines changed

‎src/api/api.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3785,7 +3785,6 @@ std::shared_ptr<v8::BackingStore> v8::ArrayBuffer::GetBackingStore() {
37853785
backing_store =
37863786
i::BackingStore::EmptyBackingStore(i::SharedFlag::kNotShared);
37873787
}
3788-
i::GlobalBackingStoreRegistry::Register(backing_store);
37893788
std::shared_ptr<i::BackingStoreBase> bs_base = backing_store;
37903789
return std::static_pointer_cast<v8::BackingStore>(bs_base);
37913790
}
@@ -3796,7 +3795,6 @@ std::shared_ptr<v8::BackingStore> v8::SharedArrayBuffer::GetBackingStore() {
37963795
if (!backing_store) {
37973796
backing_store = i::BackingStore::EmptyBackingStore(i::SharedFlag::kShared);
37983797
}
3799-
i::GlobalBackingStoreRegistry::Register(backing_store);
38003798
std::shared_ptr<i::BackingStoreBase> bs_base = backing_store;
38013799
return std::static_pointer_cast<v8::BackingStore>(bs_base);
38023800
}

‎src/objects/backing-store.cc‎

Lines changed: 5 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -685,17 +685,8 @@ inline GlobalBackingStoreRegistryImpl* impl() {
685685
void GlobalBackingStoreRegistry::Register(
686686
std::shared_ptr<BackingStore> backing_store) {
687687
if (!backing_store || !backing_store->buffer_start()) return;
688-
689-
if (!backing_store->free_on_destruct()) {
690-
// If the backing store buffer is managed by the embedder,
691-
// then we don't have to guarantee that there is single unique
692-
// BackingStore per buffer_start() because the destructor of
693-
// of the BackingStore will be a no-op in that case.
694-
695-
// All Wasm memory has to be registered.
696-
CHECK(!backing_store->is_wasm_memory());
697-
return;
698-
}
688+
// Only wasm memory backing stores need to be registered globally.
689+
CHECK(backing_store->is_wasm_memory());
699690

700691
base::MutexGuard scope_lock(&impl()->mutex_);
701692
if (backing_store->globally_registered_) return;
@@ -711,6 +702,8 @@ void GlobalBackingStoreRegistry::Register(
711702
void GlobalBackingStoreRegistry::Unregister(BackingStore* backing_store) {
712703
if (!backing_store->globally_registered_) return;
713704

705+
CHECK(backing_store->is_wasm_memory());
706+
714707
DCHECK_NOT_NULL(backing_store->buffer_start());
715708

716709
base::MutexGuard scope_lock(&impl()->mutex_);
@@ -722,26 +715,6 @@ void GlobalBackingStoreRegistry::Unregister(BackingStore* backing_store) {
722715
backing_store->globally_registered_ = false;
723716
}
724717

725-
std::shared_ptr<BackingStore> GlobalBackingStoreRegistry::Lookup(
726-
void* buffer_start, size_t length) {
727-
base::MutexGuard scope_lock(&impl()->mutex_);
728-
TRACE_BS("BS:lookup mem=%p (%zu bytes)\n", buffer_start, length);
729-
const auto& result = impl()->map_.find(buffer_start);
730-
if (result == impl()->map_.end()) {
731-
return std::shared_ptr<BackingStore>();
732-
}
733-
auto backing_store = result->second.lock();
734-
CHECK_EQ(buffer_start, backing_store->buffer_start());
735-
if (backing_store->is_wasm_memory()) {
736-
// Grow calls to shared WebAssembly threads can be triggered from different
737-
// workers, length equality cannot be guaranteed here.
738-
CHECK_LE(length, backing_store->byte_length());
739-
} else {
740-
CHECK_EQ(length, backing_store->byte_length());
741-
}
742-
return backing_store;
743-
}
744-
745718
void GlobalBackingStoreRegistry::Purge(Isolate* isolate) {
746719
// We need to keep a reference to all backing stores that are inspected
747720
// in the purging loop below. Otherwise, we might get a deadlock
@@ -755,7 +728,7 @@ void GlobalBackingStoreRegistry::Purge(Isolate* isolate) {
755728
auto backing_store = entry.second.lock();
756729
prevent_destruction_under_lock.emplace_back(backing_store);
757730
if (!backing_store) continue; // skip entries where weak ptr is null
758-
if (!backing_store->is_wasm_memory()) continue; // skip non-wasm memory
731+
CHECK(backing_store->is_wasm_memory());
759732
if (!backing_store->is_shared()) continue; // skip non-shared memory
760733
SharedWasmMemoryData* shared_data =
761734
backing_store->get_shared_wasm_memory_data();

‎src/objects/backing-store.h‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -219,21 +219,16 @@ class V8_EXPORT_PRIVATE BackingStore : public BackingStoreBase {
219219
#endif // V8_ENABLE_WEBASSEMBLY
220220
};
221221

222-
// A global, per-process mapping from buffer addresses to backing stores.
223-
// This is generally only used for dealing with an embedder that has not
224-
// migrated to the new API which should use proper pointers to manage
225-
// backing stores.
222+
// A global, per-process mapping from buffer addresses to backing stores
223+
// of wasm memory objects.
226224
class GlobalBackingStoreRegistry {
227225
public:
228226
// Register a backing store in the global registry. A mapping from the
229227
// {buffer_start} to the backing store object will be added. The backing
230228
// store will automatically unregister itself upon destruction.
229+
// Only wasm memory backing stores are supported.
231230
static void Register(std::shared_ptr<BackingStore> backing_store);
232231

233-
// Look up a backing store based on the {buffer_start} pointer.
234-
static std::shared_ptr<BackingStore> Lookup(void* buffer_start,
235-
size_t length);
236-
237232
private:
238233
friend class BackingStore;
239234
// Unregister a backing store in the global registry.

0 commit comments

Comments
 (0)