Commit 6d73ced6 authored by Jakob Kummerow's avatar Jakob Kummerow Committed by Commit Bot

[wasm-c-api] Drop Isolate::Enter/Exit calls...

...from the Store constructor/destructor. They were preventing embedders
from using several Stores with overlapping but non-nested lifetimes.
Without Isolate::Enter, such use cases are supported; the only consequence
is that Isolate::Current will not work and therefore must not be called;
but it is deprecated and not called from the Wasm C API anyway.

Change-Id: I65eda00243126e189febb0fd8b38a953c4ee078f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1698387
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62692}
parent dea2de5c
...@@ -269,7 +269,6 @@ StoreImpl::~StoreImpl() { ...@@ -269,7 +269,6 @@ StoreImpl::~StoreImpl() {
v8::kGCCallbackFlagForced); v8::kGCCallbackFlagForced);
#endif #endif
context()->Exit(); context()->Exit();
isolate_->Exit();
isolate_->Dispose(); isolate_->Dispose();
delete create_params_.array_buffer_allocator; delete create_params_.array_buffer_allocator;
} }
...@@ -294,7 +293,6 @@ auto Store::make(Engine*) -> own<Store*> { ...@@ -294,7 +293,6 @@ auto Store::make(Engine*) -> own<Store*> {
if (!isolate) return own<Store*>(); if (!isolate) return own<Store*>();
{ {
v8::Isolate::Scope isolate_scope(isolate);
v8::HandleScope handle_scope(isolate); v8::HandleScope handle_scope(isolate);
// Create context. // Create context.
...@@ -305,8 +303,10 @@ auto Store::make(Engine*) -> own<Store*> { ...@@ -305,8 +303,10 @@ auto Store::make(Engine*) -> own<Store*> {
store->isolate_ = isolate; store->isolate_ = isolate;
store->context_ = v8::Eternal<v8::Context>(isolate, context); store->context_ = v8::Eternal<v8::Context>(isolate, context);
} }
// We intentionally do not call isolate->Enter() here, because that would
store->isolate()->Enter(); // prevent embedders from using stores with overlapping but non-nested
// lifetimes. The consequence is that Isolate::Current() is dysfunctional
// and hence must not be called by anything reachable via this file.
store->context()->Enter(); store->context()->Enter();
isolate->SetData(0, store.get()); isolate->SetData(0, store.get());
......
...@@ -90,6 +90,16 @@ TEST_F(WasmCapiTest, Threads) { ...@@ -90,6 +90,16 @@ TEST_F(WasmCapiTest, Threads) {
EXPECT_EQ(kExpected, g_traces); EXPECT_EQ(kExpected, g_traces);
} }
TEST_F(WasmCapiTest, MultiStoresOneThread) {
// These Stores intentionally have overlapping, but non-nested lifetimes.
own<Store*> store1 = Store::make(engine());
own<Store*> store2 = Store::make(engine());
own<Store*> store3 = Store::make(engine());
store1.reset();
store2.reset();
store3.reset();
}
} // namespace wasm } // namespace wasm
} // namespace internal } // namespace internal
} // namespace v8 } // namespace v8
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