diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc index c456641917..13ff42d0f4 100644 --- a/src/execution/isolate.cc +++ b/src/execution/isolate.cc @@ -2692,9 +2692,10 @@ void Isolate::UnregisterManagedPtrDestructor(ManagedPtrDestructor* destructor) { } #if V8_ENABLE_WEBASSEMBLY -void Isolate::SetWasmEngine(std::shared_ptr engine) { +void Isolate::SetWasmEngine(wasm::WasmEngine* engine) { DCHECK_NULL(wasm_engine_); // Only call once before {Init}. - wasm_engine_ = std::move(engine); + DCHECK_NOT_NULL(engine); + wasm_engine_ = engine; wasm_engine_->AddIsolate(this); } @@ -3136,10 +3137,7 @@ void Isolate::Deinit() { if (logfile != nullptr) base::Fclose(logfile); #if V8_ENABLE_WEBASSEMBLY - if (wasm_engine_) { - wasm_engine_->RemoveIsolate(this); - wasm_engine_.reset(); - } + wasm_engine_->RemoveIsolate(this); #endif // V8_ENABLE_WEBASSEMBLY TearDownEmbeddedBlob(); @@ -3617,11 +3615,7 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data, isolate_data_.external_reference_table()->Init(this); #if V8_ENABLE_WEBASSEMBLY - // Setup the wasm engine. - if (wasm_engine_ == nullptr) { - SetWasmEngine(wasm::WasmEngine::GetWasmEngine()); - } - DCHECK_NOT_NULL(wasm_engine_); + SetWasmEngine(wasm::WasmEngine::GetWasmEngine()); #endif // V8_ENABLE_WEBASSEMBLY if (setup_delegate_ == nullptr) { diff --git a/src/execution/isolate.h b/src/execution/isolate.h index 061914362f..ac016520ca 100644 --- a/src/execution/isolate.h +++ b/src/execution/isolate.h @@ -1684,8 +1684,9 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { } #if V8_ENABLE_WEBASSEMBLY - wasm::WasmEngine* wasm_engine() const { return wasm_engine_.get(); } - void SetWasmEngine(std::shared_ptr engine); + // TODO(wasm): Replace all uses by {WasmEngine::GetWasmEngine}? + wasm::WasmEngine* wasm_engine() const { return wasm_engine_; } + void SetWasmEngine(wasm::WasmEngine* engine); void AddSharedWasmMemory(Handle memory_object); #endif // V8_ENABLE_WEBASSEMBLY @@ -2135,7 +2136,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { size_t elements_deletion_counter_ = 0; #if V8_ENABLE_WEBASSEMBLY - std::shared_ptr wasm_engine_; + wasm::WasmEngine* wasm_engine_ = nullptr; #endif // V8_ENABLE_WEBASSEMBLY std::unique_ptr tracing_cpu_profiler_; diff --git a/src/wasm/wasm-engine.cc b/src/wasm/wasm-engine.cc index 7f40180335..7bcebf944d 100644 --- a/src/wasm/wasm-engine.cc +++ b/src/wasm/wasm-engine.cc @@ -1561,24 +1561,29 @@ void WasmEngine::PotentiallyFinishCurrentGC() { namespace { -DEFINE_LAZY_LEAKY_OBJECT_GETTER(std::shared_ptr, - GetSharedWasmEngine) +WasmEngine* global_wasm_engine = nullptr; } // namespace // static void WasmEngine::InitializeOncePerProcess() { - *GetSharedWasmEngine() = std::make_shared(); + DCHECK_NULL(global_wasm_engine); + global_wasm_engine = new WasmEngine(); } // static void WasmEngine::GlobalTearDown() { - GetSharedWasmEngine()->reset(); + // Note: This can be called multiple times in a row (see + // test-api/InitializeAndDisposeMultiple). This is fine, as + // {global_wasm_engine} will be nullptr then. + delete global_wasm_engine; + global_wasm_engine = nullptr; } // static -std::shared_ptr WasmEngine::GetWasmEngine() { - return *GetSharedWasmEngine(); +WasmEngine* WasmEngine::GetWasmEngine() { + DCHECK_NOT_NULL(global_wasm_engine); + return global_wasm_engine; } // {max_mem_pages} is declared in wasm-limits.h. diff --git a/src/wasm/wasm-engine.h b/src/wasm/wasm-engine.h index b65310d683..d4736036cb 100644 --- a/src/wasm/wasm-engine.h +++ b/src/wasm/wasm-engine.h @@ -357,10 +357,8 @@ class V8_EXPORT_PRIVATE WasmEngine { static void InitializeOncePerProcess(); static void GlobalTearDown(); - // Returns a reference to the WasmEngine shared by the entire process. Try to - // use {Isolate::wasm_engine} instead if it is available, which encapsulates - // engine lifetime decisions during Isolate bootstrapping. - static std::shared_ptr GetWasmEngine(); + // Returns a reference to the WasmEngine shared by the entire process. + static WasmEngine* GetWasmEngine(); private: struct CurrentGCInfo; diff --git a/test/cctest/wasm/test-wasm-serialization.cc b/test/cctest/wasm/test-wasm-serialization.cc index 79ba524ffb..2f8f92b803 100644 --- a/test/cctest/wasm/test-wasm-serialization.cc +++ b/test/cctest/wasm/test-wasm-serialization.cc @@ -257,7 +257,6 @@ TEST(BlockWasmCodeGenAtDeserialization) { } UNINITIALIZED_TEST(CompiledWasmModulesTransfer) { - i::wasm::WasmEngine::InitializeOncePerProcess(); v8::internal::AccountingAllocator allocator; Zone zone(&allocator, ZONE_NAME); diff --git a/test/cctest/wasm/test-wasm-shared-engine.cc b/test/cctest/wasm/test-wasm-shared-engine.cc index b7f3a8d83e..d6d9e40a04 100644 --- a/test/cctest/wasm/test-wasm-shared-engine.cc +++ b/test/cctest/wasm/test-wasm-shared-engine.cc @@ -22,40 +22,15 @@ namespace internal { namespace wasm { namespace test_wasm_shared_engine { -// Helper class representing a WebAssembly engine that is capable of being -// shared between multiple Isolates, sharing the underlying generated code. -class SharedEngine { - public: - ~SharedEngine() { - // Ensure no remaining uses exist. - CHECK(wasm_engine_.unique()); - } - - WasmEngine* engine() const { return wasm_engine_.get(); } - WasmCodeManager* code_manager() const { return engine()->code_manager(); } - - int NumberOfExportedEngineUses() const { - // This class holds one implicit use itself, which we discount. - return static_cast(wasm_engine_.use_count()) - 1; - } - - std::shared_ptr ExportEngineForSharing() { return wasm_engine_; } - - private: - std::shared_ptr wasm_engine_ = std::make_unique(); -}; - // Helper type definition representing a WebAssembly module shared between // multiple Isolates with implicit reference counting. using SharedModule = std::shared_ptr; -// Helper class representing an Isolate based on a given shared WebAssembly -// engine available at construction time. +// Helper class representing an Isolate that uses the process-wide (shared) wasm +// engine. class SharedEngineIsolate { public: - explicit SharedEngineIsolate(SharedEngine* engine) - : isolate_(v8::Isolate::Allocate()) { - isolate()->SetWasmEngine(engine->ExportEngineForSharing()); + SharedEngineIsolate() : isolate_(v8::Isolate::Allocate()) { v8::Isolate::CreateParams create_params; create_params.array_buffer_allocator = CcTest::array_buffer_allocator(); v8::Isolate::Initialize(isolate_, create_params); @@ -111,19 +86,16 @@ class SharedEngineIsolate { // with a shared WebAssembly engine available at construction time. class SharedEngineThread : public v8::base::Thread { public: - SharedEngineThread(SharedEngine* engine, - std::function callback) - : Thread(Options("SharedEngineThread")), - engine_(engine), - callback_(callback) {} + explicit SharedEngineThread( + std::function callback) + : Thread(Options("SharedEngineThread")), callback_(callback) {} void Run() override { - SharedEngineIsolate isolate(engine_); + SharedEngineIsolate isolate; callback_(&isolate); } private: - SharedEngine* engine_; std::function callback_; }; @@ -201,34 +173,16 @@ Handle CompileAndInstantiateAsync( } // namespace -TEST(SharedEngineUseCount) { - SharedEngine engine; - CHECK_EQ(0, engine.NumberOfExportedEngineUses()); - { - SharedEngineIsolate isolate(&engine); - CHECK_EQ(1, engine.NumberOfExportedEngineUses()); - } - CHECK_EQ(0, engine.NumberOfExportedEngineUses()); - { - SharedEngineIsolate isolate1(&engine); - CHECK_EQ(1, engine.NumberOfExportedEngineUses()); - SharedEngineIsolate isolate2(&engine); - CHECK_EQ(2, engine.NumberOfExportedEngineUses()); - } - CHECK_EQ(0, engine.NumberOfExportedEngineUses()); -} - TEST(SharedEngineRunSeparated) { - SharedEngine engine; { - SharedEngineIsolate isolate(&engine); + SharedEngineIsolate isolate; HandleScope scope(isolate.isolate()); ZoneBuffer* buffer = BuildReturnConstantModule(isolate.zone(), 23); Handle instance = isolate.CompileAndInstantiate(buffer); CHECK_EQ(23, isolate.Run(instance)); } { - SharedEngineIsolate isolate(&engine); + SharedEngineIsolate isolate; HandleScope scope(isolate.isolate()); ZoneBuffer* buffer = BuildReturnConstantModule(isolate.zone(), 42); Handle instance = isolate.CompileAndInstantiate(buffer); @@ -237,10 +191,9 @@ TEST(SharedEngineRunSeparated) { } TEST(SharedEngineRunImported) { - SharedEngine engine; SharedModule module; { - SharedEngineIsolate isolate(&engine); + SharedEngineIsolate isolate; HandleScope scope(isolate.isolate()); ZoneBuffer* buffer = BuildReturnConstantModule(isolate.zone(), 23); Handle instance = isolate.CompileAndInstantiate(buffer); @@ -248,7 +201,7 @@ TEST(SharedEngineRunImported) { CHECK_EQ(23, isolate.Run(instance)); } { - SharedEngineIsolate isolate(&engine); + SharedEngineIsolate isolate; HandleScope scope(isolate.isolate()); Handle instance = isolate.ImportInstance(module); CHECK_EQ(23, isolate.Run(instance)); @@ -256,15 +209,14 @@ TEST(SharedEngineRunImported) { } TEST(SharedEngineRunThreadedBuildingSync) { - SharedEngine engine; - SharedEngineThread thread1(&engine, [](SharedEngineIsolate* isolate) { + SharedEngineThread thread1([](SharedEngineIsolate* isolate) { HandleScope scope(isolate->isolate()); ZoneBuffer* buffer = BuildReturnConstantModule(isolate->zone(), 23); Handle instance = isolate->CompileAndInstantiate(buffer); CHECK_EQ(23, isolate->Run(instance)); }); - SharedEngineThread thread2(&engine, [](SharedEngineIsolate* isolate) { + SharedEngineThread thread2([](SharedEngineIsolate* isolate) { HandleScope scope(isolate->isolate()); ZoneBuffer* buffer = BuildReturnConstantModule(isolate->zone(), 42); Handle instance = @@ -278,15 +230,14 @@ TEST(SharedEngineRunThreadedBuildingSync) { } TEST(SharedEngineRunThreadedBuildingAsync) { - SharedEngine engine; - SharedEngineThread thread1(&engine, [](SharedEngineIsolate* isolate) { + SharedEngineThread thread1([](SharedEngineIsolate* isolate) { HandleScope scope(isolate->isolate()); ZoneBuffer* buffer = BuildReturnConstantModule(isolate->zone(), 23); Handle instance = CompileAndInstantiateAsync(isolate, buffer); CHECK_EQ(23, isolate->Run(instance)); }); - SharedEngineThread thread2(&engine, [](SharedEngineIsolate* isolate) { + SharedEngineThread thread2([](SharedEngineIsolate* isolate) { HandleScope scope(isolate->isolate()); ZoneBuffer* buffer = BuildReturnConstantModule(isolate->zone(), 42); Handle instance = @@ -300,21 +251,20 @@ TEST(SharedEngineRunThreadedBuildingAsync) { } TEST(SharedEngineRunThreadedExecution) { - SharedEngine engine; SharedModule module; { - SharedEngineIsolate isolate(&engine); + SharedEngineIsolate isolate; HandleScope scope(isolate.isolate()); ZoneBuffer* buffer = BuildReturnConstantModule(isolate.zone(), 23); Handle instance = isolate.CompileAndInstantiate(buffer); module = isolate.ExportInstance(instance); } - SharedEngineThread thread1(&engine, [module](SharedEngineIsolate* isolate) { + SharedEngineThread thread1([module](SharedEngineIsolate* isolate) { HandleScope scope(isolate->isolate()); Handle instance = isolate->ImportInstance(module); CHECK_EQ(23, isolate->Run(instance)); }); - SharedEngineThread thread2(&engine, [module](SharedEngineIsolate* isolate) { + SharedEngineThread thread2([module](SharedEngineIsolate* isolate) { HandleScope scope(isolate->isolate()); Handle instance = isolate->ImportInstance(module); CHECK_EQ(23, isolate->Run(instance)); @@ -326,10 +276,9 @@ TEST(SharedEngineRunThreadedExecution) { } TEST(SharedEngineRunThreadedTierUp) { - SharedEngine engine; SharedModule module; { - SharedEngineIsolate isolate(&engine); + SharedEngineIsolate isolate; HandleScope scope(isolate.isolate()); ZoneBuffer* buffer = BuildReturnConstantModule(isolate.zone(), 23); Handle instance = isolate.CompileAndInstantiate(buffer); @@ -338,7 +287,7 @@ TEST(SharedEngineRunThreadedTierUp) { constexpr int kNumberOfThreads = 5; std::list threads; for (int i = 0; i < kNumberOfThreads; ++i) { - threads.emplace_back(&engine, [module](SharedEngineIsolate* isolate) { + threads.emplace_back([module](SharedEngineIsolate* isolate) { constexpr int kNumberOfIterations = 100; HandleScope scope(isolate->isolate()); Handle instance = isolate->ImportInstance(module); @@ -347,7 +296,7 @@ TEST(SharedEngineRunThreadedTierUp) { } }); } - threads.emplace_back(&engine, [module](SharedEngineIsolate* isolate) { + threads.emplace_back([module](SharedEngineIsolate* isolate) { HandleScope scope(isolate->isolate()); Handle instance = isolate->ImportInstance(module); WasmFeatures detected = WasmFeatures::None();