From 3aa4f05d0c6422f9d5525a857e778490d2801213 Mon Sep 17 00:00:00 2001 From: Jakob Kummerow Date: Mon, 26 Aug 2019 14:56:05 +0200 Subject: [PATCH] [wasm-c-api] Roll bf31edf: Fix life times of host info Host info used to be stored on the global reference underlying a Ref; now it is stored in a JSWeakMap and hence tied to the lifetime of the actual object on V8's heap. Additionally, the internal metadata needed for C-API functions is now stored on the SharedFunctionInfo and no longer overlaps with the host info mechanism. Bonus content: Roll 6db391e: Remove a few more leftover uses of _enum types Change-Id: Ibb1fa4b0dd5157fef15c030bac705a11aa3beaea Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1768368 Reviewed-by: Andreas Haas Commit-Queue: Jakob Kummerow Cr-Commit-Position: refs/heads/master@{#63400} --- src/builtins/base.tq | 2 +- src/compiler/wasm-compiler.cc | 9 +- src/diagnostics/objects-printer.cc | 2 +- src/wasm/c-api.cc | 124 +++++++++---------- src/wasm/c-api.h | 14 +++ src/wasm/wasm-objects-inl.h | 3 +- src/wasm/wasm-objects.cc | 4 +- src/wasm/wasm-objects.h | 6 +- test/wasm-api-tests/finalize.cc | 144 ++++++++++++++++++++--- test/wasm-api-tests/wasm-api-test.h | 1 + third_party/wasm-api/README.v8 | 4 +- third_party/wasm-api/example/finalize.c | 55 +++++++-- third_party/wasm-api/example/finalize.cc | 47 ++++++-- 13 files changed, 305 insertions(+), 110 deletions(-) diff --git a/src/builtins/base.tq b/src/builtins/base.tq index 9681a6a84b..28d1fc80f2 100644 --- a/src/builtins/base.tq +++ b/src/builtins/base.tq @@ -1000,7 +1000,7 @@ extern class WasmJSFunctionData extends Struct { extern class WasmCapiFunctionData extends Struct { call_target: RawPtr; - embedder_data: RawPtr; + embedder_data: Foreign; // Managed wrapper_code: Code; serialized_signature: ByteArray; // PodArray } diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc index 3a443cc8df..7e05303f14 100644 --- a/src/compiler/wasm-compiler.cc +++ b/src/compiler/wasm-compiler.cc @@ -5952,9 +5952,9 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder { Node* sfi_data = LOAD_RAW( shared, SharedFunctionInfo::kFunctionDataOffset - kHeapObjectTag, MachineType::TypeCompressedTagged()); - Node* host_data = LOAD_RAW( + Node* host_data_foreign = LOAD_RAW( sfi_data, WasmCapiFunctionData::kEmbedderDataOffset - kHeapObjectTag, - MachineType::Pointer()); + MachineType::TypeCompressedTagged()); BuildModifyThreadInWasmFlag(false); Node* isolate_root = BuildLoadIsolateRoot(); @@ -5968,11 +5968,12 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder { Node* function = graph()->NewNode(mcgraph()->common()->ExternalConstant(ref)); - // Parameters: void* data, Address arguments. + // Parameters: Address host_data_foreign, Address arguments. MachineType host_sig_types[] = { MachineType::Pointer(), MachineType::Pointer(), MachineType::Pointer()}; MachineSignature host_sig(1, 2, host_sig_types); - Node* return_value = BuildCCall(&host_sig, function, host_data, values); + Node* return_value = + BuildCCall(&host_sig, function, host_data_foreign, values); BuildModifyThreadInWasmFlag(true); diff --git a/src/diagnostics/objects-printer.cc b/src/diagnostics/objects-printer.cc index 4e0362203f..b31a05c486 100644 --- a/src/diagnostics/objects-printer.cc +++ b/src/diagnostics/objects-printer.cc @@ -2061,7 +2061,7 @@ void WasmCapiFunctionData::WasmCapiFunctionDataPrint( std::ostream& os) { // NOLINT PrintHeader(os, "WasmCapiFunctionData"); os << "\n - call_target: " << call_target(); - os << "\n - embedder_data: " << embedder_data(); + os << "\n - embedder_data: " << Brief(embedder_data()); os << "\n - wrapper_code: " << Brief(wrapper_code()); os << "\n - serialized_signature: " << Brief(serialized_signature()); os << "\n"; diff --git a/src/wasm/c-api.cc b/src/wasm/c-api.cc index ebac1327ab..50002e838f 100644 --- a/src/wasm/c-api.cc +++ b/src/wasm/c-api.cc @@ -29,6 +29,8 @@ #include "include/libplatform/libplatform.h" #include "src/api/api-inl.h" #include "src/compiler/wasm-compiler.h" +#include "src/objects/js-collection-inl.h" +#include "src/objects/managed.h" #include "src/objects/stack-frame-info-inl.h" #include "src/wasm/leb-helper.h" #include "src/wasm/module-instantiate.h" @@ -270,6 +272,38 @@ StoreImpl::~StoreImpl() { delete create_params_.array_buffer_allocator; } +struct ManagedData { + ManagedData(void* info, void (*finalizer)(void*)) + : info(info), finalizer(finalizer) {} + + ~ManagedData() { + if (finalizer) (*finalizer)(info); + } + + void* info; + void (*finalizer)(void*); +}; + +void StoreImpl::SetHostInfo(i::Handle object, void* info, + void (*finalizer)(void*)) { + i::HandleScope scope(i_isolate()); + // Ideally we would specify the total size kept alive by {info} here, + // but all we get from the embedder is a {void*}, so our best estimate + // is the size of the metadata. + size_t estimated_size = sizeof(ManagedData); + i::Handle wrapper = i::Managed::FromRawPtr( + i_isolate(), estimated_size, new ManagedData(info, finalizer)); + int32_t hash = object->GetOrCreateHash(i_isolate()).value(); + i::JSWeakCollection::Set(host_info_map_, object, wrapper, hash); +} + +void* StoreImpl::GetHostInfo(i::Handle key) { + i::Object raw = + i::EphemeronHashTable::cast(host_info_map_->table()).Lookup(key); + if (raw.IsTheHole(i_isolate())) return nullptr; + return i::Managed::cast(raw).raw()->info; +} + template <> struct implement { using type = StoreImpl; @@ -286,26 +320,29 @@ auto Store::make(Engine*) -> own { // Create isolate. store->create_params_.array_buffer_allocator = v8::ArrayBuffer::Allocator::NewDefaultAllocator(); - auto isolate = v8::Isolate::New(store->create_params_); + v8::Isolate* isolate = v8::Isolate::New(store->create_params_); if (!isolate) return own(); + store->isolate_ = isolate; + isolate->SetData(0, store.get()); + // We intentionally do not call isolate->Enter() here, because that would + // 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. { v8::HandleScope handle_scope(isolate); // Create context. - auto context = v8::Context::New(isolate); + v8::Local context = v8::Context::New(isolate); if (context.IsEmpty()) return own(); - v8::Context::Scope context_scope(context); - - store->isolate_ = isolate; + context->Enter(); // The Exit() call is in ~StoreImpl. store->context_ = v8::Eternal(isolate, context); + + // Create weak map for Refs with host info. + i::Isolate* i_isolate = reinterpret_cast(isolate); + store->host_info_map_ = i_isolate->global_handles()->Create( + *i_isolate->factory()->NewJSWeakMap()); } - // We intentionally do not call isolate->Enter() here, because that would - // 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(); - isolate->SetData(0, store.get()); // We want stack traces for traps. constexpr int kStackLimit = 10; isolate->SetCaptureStackTraceForUncaughtExceptions(true, kStackLimit, @@ -712,15 +749,7 @@ class RefImpl { return make_own(seal(self)); } - void Reset() { - i::GlobalHandles::Destroy(location()); - if (host_data_) { - if (host_data_->finalizer) { - host_data_->finalizer(host_data_->info); - } - delete host_data_; - } - } + ~RefImpl() { i::GlobalHandles::Destroy(location()); } own copy() const { return make(store(), v8_object()); } @@ -730,41 +759,20 @@ class RefImpl { i::Handle v8_object() const { return i::Handle::cast(val_); } - void* get_host_info() const { - if (host_data_ == nullptr) return nullptr; - return host_data_->info; - } + void* get_host_info() const { return store()->GetHostInfo(v8_object()); } void set_host_info(void* info, void (*finalizer)(void*)) { - host_data_ = new HostData(location(), info, finalizer); - i::GlobalHandles::MakeWeak(host_data_->location, host_data_, &v8_finalizer, - v8::WeakCallbackType::kParameter); + store()->SetHostInfo(v8_object(), info, finalizer); } private: - struct HostData { - HostData(i::Address* location, void* info, void (*finalizer)(void*)) - : location(location), info(info), finalizer(finalizer) {} - i::Address* location; - void* info; - void (*finalizer)(void*); - }; - RefImpl() {} - static void v8_finalizer(const v8::WeakCallbackInfo& info) { - HostData* data = reinterpret_cast(info.GetParameter()); - i::GlobalHandles::Destroy(data->location); - if (data->finalizer) (*data->finalizer)(data->info); - delete data; - } - i::Address* location() const { return reinterpret_cast(val_.address()); } i::Handle val_; - HostData* host_data_ = nullptr; }; template <> @@ -773,7 +781,6 @@ struct implement { }; Ref::~Ref() { - impl(this)->Reset(); delete impl(this); } @@ -1197,8 +1204,7 @@ struct FuncData { if (finalizer) (*finalizer)(env); } - static i::Address v8_callback(void* data, i::Address argv); - static void finalize_func_data(void* data); + static i::Address v8_callback(i::Address host_data_foreign, i::Address argv); }; namespace { @@ -1270,11 +1276,12 @@ auto make_func(Store* store_abs, FuncData* data) -> own { auto store = impl(store_abs); i::Isolate* isolate = store->i_isolate(); i::HandleScope handle_scope(isolate); + i::Handle> embedder_data = + i::Managed::FromRawPtr(isolate, sizeof(FuncData), data); i::Handle function = i::WasmCapiFunction::New( - isolate, reinterpret_cast(&FuncData::v8_callback), data, - SignatureHelper::Serialize(isolate, data->type.get())); + isolate, reinterpret_cast(&FuncData::v8_callback), + embedder_data, SignatureHelper::Serialize(isolate, data->type.get())); auto func = implement::type::make(store, function); - func->set_host_info(data, &FuncData::finalize_func_data); return func; } @@ -1435,7 +1442,7 @@ void PopArgs(i::wasm::FunctionSig* sig, Val results[], own CallWasmCapiFunction(i::WasmCapiFunctionData data, const Val args[], Val results[]) { - FuncData* func_data = reinterpret_cast(data.embedder_data()); + FuncData* func_data = i::Managed::cast(data.embedder_data()).raw(); if (func_data->kind == FuncData::kCallback) { return (func_data->callback)(args, results); } @@ -1530,8 +1537,10 @@ auto Func::call(const Val args[], Val results[]) const -> own { return nullptr; } -i::Address FuncData::v8_callback(void* data, i::Address argv) { - FuncData* self = reinterpret_cast(data); +i::Address FuncData::v8_callback(i::Address host_data_foreign, + i::Address argv) { + FuncData* self = + i::Managed::cast(i::Object(host_data_foreign))->raw(); StoreImpl* store = impl(self->store); i::Isolate* isolate = store->i_isolate(); i::HandleScope scope(isolate); @@ -1619,10 +1628,6 @@ i::Address FuncData::v8_callback(void* data, i::Address argv) { return i::kNullAddress; } -void FuncData::finalize_func_data(void* data) { - delete reinterpret_cast(data); -} - // Global Instances template <> @@ -2251,7 +2256,7 @@ extern "C++" inline auto hide_mutability(wasm::Mutability mutability) return static_cast(mutability); } -extern "C++" inline auto reveal_mutability(wasm_mutability_enum mutability) +extern "C++" inline auto reveal_mutability(wasm_mutability_t mutability) -> wasm::Mutability { return static_cast(mutability); } @@ -2278,7 +2283,7 @@ extern "C++" inline auto hide_externkind(wasm::ExternKind kind) return static_cast(kind); } -extern "C++" inline auto reveal_externkind(wasm_externkind_enum kind) +extern "C++" inline auto reveal_externkind(wasm_externkind_t kind) -> wasm::ExternKind { return static_cast(kind); } @@ -2330,8 +2335,7 @@ WASM_DEFINE_TYPE(globaltype, wasm::GlobalType) wasm_globaltype_t* wasm_globaltype_new(wasm_valtype_t* content, wasm_mutability_t mutability) { return release_globaltype(wasm::GlobalType::make( - adopt_valtype(content), - reveal_mutability(static_cast(mutability)))); + adopt_valtype(content), reveal_mutability(mutability))); } const wasm_valtype_t* wasm_globaltype_content(const wasm_globaltype_t* gt) { diff --git a/src/wasm/c-api.h b/src/wasm/c-api.h index cdd9d4dfd5..43a0fb73b2 100644 --- a/src/wasm/c-api.h +++ b/src/wasm/c-api.h @@ -7,8 +7,17 @@ #include "include/v8.h" #include "src/common/globals.h" +#include "src/handles/handles.h" #include "third_party/wasm-api/wasm.hh" +namespace v8 { +namespace internal { + +class JSWeakMap; + +} // namespace internal +} // namespace v8 + namespace wasm { class StoreImpl { @@ -27,6 +36,10 @@ class StoreImpl { reinterpret_cast(isolate)->GetData(0)); } + void SetHostInfo(i::Handle object, void* info, + void (*finalizer)(void*)); + void* GetHostInfo(i::Handle key); + private: friend own Store::make(Engine*); @@ -35,6 +48,7 @@ class StoreImpl { v8::Isolate::CreateParams create_params_; v8::Isolate* isolate_ = nullptr; v8::Eternal context_; + i::Handle host_info_map_; }; } // namespace wasm diff --git a/src/wasm/wasm-objects-inl.h b/src/wasm/wasm-objects-inl.h index ab8aad4490..43e4c6cbd2 100644 --- a/src/wasm/wasm-objects-inl.h +++ b/src/wasm/wasm-objects-inl.h @@ -357,8 +357,7 @@ OBJECT_CONSTRUCTORS_IMPL(WasmCapiFunctionData, Struct) CAST_ACCESSOR(WasmCapiFunctionData) PRIMITIVE_ACCESSORS(WasmCapiFunctionData, call_target, Address, kCallTargetOffset) -PRIMITIVE_ACCESSORS(WasmCapiFunctionData, embedder_data, void*, - kEmbedderDataOffset) +ACCESSORS(WasmCapiFunctionData, embedder_data, Foreign, kEmbedderDataOffset) ACCESSORS(WasmCapiFunctionData, wrapper_code, Code, kWrapperCodeOffset) ACCESSORS(WasmCapiFunctionData, serialized_signature, PodArray, kSerializedSignatureOffset) diff --git a/src/wasm/wasm-objects.cc b/src/wasm/wasm-objects.cc index a8b3e51d91..3142f8bf98 100644 --- a/src/wasm/wasm-objects.cc +++ b/src/wasm/wasm-objects.cc @@ -2162,13 +2162,13 @@ bool WasmCapiFunction::IsWasmCapiFunction(Object object) { } Handle WasmCapiFunction::New( - Isolate* isolate, Address call_target, void* embedder_data, + Isolate* isolate, Address call_target, Handle embedder_data, Handle> serialized_signature) { Handle fun_data = Handle::cast(isolate->factory()->NewStruct( WASM_CAPI_FUNCTION_DATA_TYPE, AllocationType::kOld)); fun_data->set_call_target(call_target); - fun_data->set_embedder_data(embedder_data); + fun_data->set_embedder_data(*embedder_data); fun_data->set_serialized_signature(*serialized_signature); // TODO(jkummerow): Install a JavaScript wrapper. For now, calling // these functions directly is unsupported; they can only be called diff --git a/src/wasm/wasm-objects.h b/src/wasm/wasm-objects.h index 1e40e855d6..066cef988f 100644 --- a/src/wasm/wasm-objects.h +++ b/src/wasm/wasm-objects.h @@ -707,7 +707,7 @@ class WasmCapiFunction : public JSFunction { static bool IsWasmCapiFunction(Object object); static Handle New( - Isolate* isolate, Address call_target, void* embedder_data, + Isolate* isolate, Address call_target, Handle embedder_data, Handle> serialized_signature); Address GetHostCallTarget() const; @@ -764,7 +764,7 @@ class WasmIndirectFunctionTable : public Struct { class WasmCapiFunctionData : public Struct { public: DECL_PRIMITIVE_ACCESSORS(call_target, Address) - DECL_PRIMITIVE_ACCESSORS(embedder_data, void*) + DECL_ACCESSORS(embedder_data, Foreign) DECL_ACCESSORS(wrapper_code, Code) DECL_ACCESSORS(serialized_signature, PodArray) @@ -776,7 +776,7 @@ class WasmCapiFunctionData : public Struct { DEFINE_FIELD_OFFSET_CONSTANTS(HeapObject::kHeaderSize, TORQUE_GENERATED_WASM_CAPI_FUNCTION_DATA_FIELDS) - STATIC_ASSERT(kStartOfStrongFieldsOffset == kWrapperCodeOffset); + STATIC_ASSERT(kStartOfStrongFieldsOffset == kEmbedderDataOffset); using BodyDescriptor = FlexibleBodyDescriptor; OBJECT_CONSTRUCTORS(WasmCapiFunctionData, Struct); diff --git a/test/wasm-api-tests/finalize.cc b/test/wasm-api-tests/finalize.cc index 8b3190e1b5..93da85460e 100644 --- a/test/wasm-api-tests/finalize.cc +++ b/test/wasm-api-tests/finalize.cc @@ -15,6 +15,8 @@ int g_functions_finalized = 0; int g_foreigns_finalized = 0; int g_modules_finalized = 0; +const int kModuleMagic = 42; + void FinalizeInstance(void* data) { int iteration = static_cast(reinterpret_cast(data)); g_instances_finalized += iteration; @@ -34,6 +36,31 @@ void FinalizeModule(void* data) { g_modules_finalized += static_cast(reinterpret_cast(data)); } +void RunInStore(Store* store, ZoneBuffer* wire_bytes, int iterations) { + size_t size = wire_bytes->end() - wire_bytes->begin(); + vec binary = vec::make( + size, reinterpret_cast(const_cast(wire_bytes->begin()))); + own module = Module::make(store, binary); + module->set_host_info(reinterpret_cast(kModuleMagic), &FinalizeModule); + for (int iteration = 0; iteration < iterations; iteration++) { + void* finalizer_data = reinterpret_cast(iteration); + own instance = Instance::make(store, module.get(), nullptr); + EXPECT_NE(nullptr, instance.get()); + instance->set_host_info(finalizer_data, &FinalizeInstance); + + own func = instance->exports()[0]->func()->copy(); + ASSERT_NE(func, nullptr); + func->set_host_info(finalizer_data, &FinalizeFunction); + Val args[] = {Val::i32(iteration)}; + Val results[1]; + func->call(args, results); + EXPECT_EQ(iteration, results[0].i32()); + + own foreign = Foreign::make(store); + foreign->set_host_info(finalizer_data, &FinalizeForeign); + } +} + } // namespace TEST_F(WasmCapiTest, InstanceFinalization) { @@ -45,31 +72,114 @@ TEST_F(WasmCapiTest, InstanceFinalization) { g_functions_finalized = 0; g_foreigns_finalized = 0; g_modules_finalized = 0; - module()->set_host_info(reinterpret_cast(42), &FinalizeModule); + module()->set_host_info(reinterpret_cast(kModuleMagic), + &FinalizeModule); static const int kIterations = 10; - for (int iteration = 0; iteration < kIterations; iteration++) { - void* finalizer_data = reinterpret_cast(iteration); - own instance = Instance::make(store(), module(), nullptr); - EXPECT_NE(nullptr, instance.get()); - instance->set_host_info(finalizer_data, &FinalizeInstance); - - own func = instance->exports()[0]->func()->copy(); - ASSERT_NE(func, nullptr); - func->set_host_info(finalizer_data, &FinalizeFunction); - - own foreign = Foreign::make(store()); - foreign->set_host_info(finalizer_data, &FinalizeForeign); + RunInStore(store(), wire_bytes(), kIterations); + { + own store2 = Store::make(engine()); + RunInStore(store2.get(), wire_bytes(), kIterations); } + RunInStore(store(), wire_bytes(), kIterations); Shutdown(); // Verify that (1) all finalizers were called, and (2) they passed the // correct host data: the loop above sets {i} as data, and the finalizer - // callbacks add them all up, so the expected value is - // sum([0, 1, ..., kIterations - 1]), which per Gauss's formula is: - static const int kExpected = (kIterations * (kIterations - 1)) / 2; + // callbacks add them all up, so the expected value after three rounds is + // 3 * sum([0, 1, ..., kIterations - 1]), which per Gauss's formula is: + static const int kExpected = 3 * ((kIterations * (kIterations - 1)) / 2); EXPECT_EQ(g_instances_finalized, kExpected); + // There are two functions per iteration. EXPECT_EQ(g_functions_finalized, kExpected); EXPECT_EQ(g_foreigns_finalized, kExpected); - EXPECT_EQ(g_modules_finalized, 42); + EXPECT_EQ(g_modules_finalized, 4 * kModuleMagic); +} + +namespace { + +own CapiFunction(void* env, const Val args[], Val results[]) { + int offset = static_cast(reinterpret_cast(env)); + results[0] = Val::i32(offset + args[0].i32()); + return nullptr; +} + +int g_host_data_finalized = 0; +int g_capi_function_finalized = 0; + +void FinalizeCapiFunction(void* data) { + int value = static_cast(reinterpret_cast(data)); + g_capi_function_finalized += value; +} + +void FinalizeHostData(void* data) { + g_host_data_finalized += static_cast(reinterpret_cast(data)); +} + +} // namespace + +TEST_F(WasmCapiTest, CapiFunctionLifetimes) { + uint32_t func_index = builder()->AddImport(CStrVector("f"), wasm_i_i_sig()); + builder()->ExportImportedFunction(CStrVector("f"), func_index); + Compile(); + own instance; + void* kHostData = reinterpret_cast(1234); + int base_summand = 1000; + { + // Test that the own<> pointers for Func and FuncType can go out of scope + // without affecting the ability of the Func to be called later. + own capi_func_type = + FuncType::make(ownvec::make(ValType::make(::wasm::I32)), + ownvec::make(ValType::make(::wasm::I32))); + own capi_func = + Func::make(store(), capi_func_type.get(), &CapiFunction, + reinterpret_cast(base_summand)); + Extern* imports[] = {capi_func.get()}; + instance = Instance::make(store(), module(), imports); + // TODO(jkummerow): It may or may not be desirable to be able to set + // host data even here and have it survive the import/export dance. + // We are awaiting resolution of the discussion at: + // https://github.com/WebAssembly/wasm-c-api/issues/100 + } + { + ownvec exports = instance->exports(); + Func* exported_func = exports[0]->func(); + constexpr int kArg = 123; + Val args[] = {Val::i32(kArg)}; + Val results[1]; + exported_func->call(args, results); + EXPECT_EQ(base_summand + kArg, results[0].i32()); + // Host data should survive destruction of the own<> pointer. + exported_func->set_host_info(kHostData); + } + { + ownvec exports = instance->exports(); + Func* exported_func = exports[0]->func(); + EXPECT_EQ(kHostData, exported_func->get_host_info()); + } + // Test that a Func can have its own internal metadata, an {env}, and + // separate {host info}, without any of that interfering with each other. + g_host_data_finalized = 0; + g_capi_function_finalized = 0; + base_summand = 23; + constexpr int kFinalizerData = 345; + { + own capi_func_type = + FuncType::make(ownvec::make(ValType::make(::wasm::I32)), + ownvec::make(ValType::make(::wasm::I32))); + own capi_func = Func::make( + store(), capi_func_type.get(), &CapiFunction, + reinterpret_cast(base_summand), &FinalizeCapiFunction); + capi_func->set_host_info(reinterpret_cast(kFinalizerData), + &FinalizeHostData); + constexpr int kArg = 19; + Val args[] = {Val::i32(kArg)}; + Val results[1]; + capi_func->call(args, results); + EXPECT_EQ(base_summand + kArg, results[0].i32()); + } + instance.reset(); + Shutdown(); + EXPECT_EQ(base_summand, g_capi_function_finalized); + EXPECT_EQ(kFinalizerData, g_host_data_finalized); } } // namespace wasm diff --git a/test/wasm-api-tests/wasm-api-test.h b/test/wasm-api-tests/wasm-api-test.h index a80f16d423..6fc70ef5f4 100644 --- a/test/wasm-api-tests/wasm-api-test.h +++ b/test/wasm-api-tests/wasm-api-test.h @@ -115,6 +115,7 @@ class WasmCapiTest : public ::testing::Test { } void Shutdown() { + exports_.reset(); instance_.reset(); module_.reset(); store_.reset(); diff --git a/third_party/wasm-api/README.v8 b/third_party/wasm-api/README.v8 index ea957620b0..bebe47b665 100644 --- a/third_party/wasm-api/README.v8 +++ b/third_party/wasm-api/README.v8 @@ -2,8 +2,8 @@ Name: Wasm C/C++ API Short Name: wasm-c-api URL: https://github.com/WebAssembly/wasm-c-api/ Version: 0 -Revision: 5c742b048f7766a0c00be3a7af23fb71ba816026 -Date: 2019-03-18 +Revision: 6db391ee7121a0695602945d11001ea3e00b0afb +Date: 2019-08-08 License: Apache 2.0 License File: LICENSE Security Critical: yes diff --git a/third_party/wasm-api/example/finalize.c b/third_party/wasm-api/example/finalize.c index 84e569a4b6..247368f28e 100644 --- a/third_party/wasm-api/example/finalize.c +++ b/third_party/wasm-api/example/finalize.c @@ -9,23 +9,21 @@ const int iterations = 100000; +int live_count = 0; + void finalize(void* data) { int i = (int)data; if (i % (iterations / 10) == 0) printf("Finalizing #%d...\n", i); + --live_count; } -int main(int argc, const char* argv[]) { - // Initialize. - printf("Initializing...\n"); - wasm_engine_t* engine = wasm_engine_new(); - wasm_store_t* store = wasm_store_new(engine); - +void run_in_store(wasm_store_t* store) { // Load binary. printf("Loading binary...\n"); FILE* file = fopen("finalize.wasm", "r"); if (!file) { printf("> Error loading module!\n"); - return 1; + exit(1); } fseek(file, 0L, SEEK_END); size_t file_size = ftell(file); @@ -34,7 +32,7 @@ int main(int argc, const char* argv[]) { wasm_byte_vec_new_uninitialized(&binary, file_size); if (fread(binary.data, file_size, 1, file) != 1) { printf("> Error loading module!\n"); - return 1; + exit(1); } fclose(file); @@ -43,7 +41,7 @@ int main(int argc, const char* argv[]) { own wasm_module_t* module = wasm_module_new(store, &binary); if (!module) { printf("> Error compiling module!\n"); - return 1; + exit(1); } wasm_byte_vec_delete(&binary); @@ -56,18 +54,53 @@ int main(int argc, const char* argv[]) { wasm_instance_new(store, module, NULL, NULL); if (!instance) { printf("> Error instantiating module %d!\n", i); - return 1; + exit(1); } void* data = (void*)(intptr_t)i; wasm_instance_set_host_info_with_finalizer(instance, data, &finalize); wasm_instance_delete(instance); + ++live_count; } wasm_module_delete(module); +} + +int main(int argc, const char* argv[]) { + // Initialize. + printf("Initializing...\n"); + wasm_engine_t* engine = wasm_engine_new(); + + printf("Live count %d\n", live_count); + printf("Creating store 1...\n"); + wasm_store_t* store1 = wasm_store_new(engine); + + printf("Running in store 1...\n"); + run_in_store(store1); + printf("Live count %d\n", live_count); + + printf("Creating store 2...\n"); + wasm_store_t* store2 = wasm_store_new(engine); + + printf("Running in store 2...\n"); + run_in_store(store2); + printf("Live count %d\n", live_count); + + printf("Deleting store 2...\n"); + wasm_store_delete(store2); + printf("Live count %d\n", live_count); + + printf("Running in store 1...\n"); + run_in_store(store1); + printf("Live count %d\n", live_count); + + printf("Deleting store 1...\n"); + wasm_store_delete(store1); + printf("Live count %d\n", live_count); + + assert(live_count == 0); // Shut down. printf("Shutting down...\n"); - wasm_store_delete(store); wasm_engine_delete(engine); // All done. diff --git a/third_party/wasm-api/example/finalize.cc b/third_party/wasm-api/example/finalize.cc index b63ac1206c..64e134b8d8 100644 --- a/third_party/wasm-api/example/finalize.cc +++ b/third_party/wasm-api/example/finalize.cc @@ -9,20 +9,17 @@ const int iterations = 100000; +int live_count = 0; + void finalize(void* data) { intptr_t i = reinterpret_cast(data); if (i % (iterations / 10) == 0) { std::cout << "Finalizing #" << i << "..." << std::endl; } + --live_count; } -void run() { - // Initialize. - std::cout << "Initializing..." << std::endl; - auto engine = wasm::Engine::make(); - auto store_ = wasm::Store::make(engine.get()); - auto store = store_.get(); - +void run_in_store(wasm::Store* store) { // Load binary. std::cout << "Loading binary..." << std::endl; std::ifstream file("finalize.wasm"); @@ -55,6 +52,7 @@ void run() { exit(1); } instance->set_host_info(reinterpret_cast(i), &finalize); + ++live_count; } // Shut down. @@ -62,8 +60,43 @@ void run() { } +void run() { + // Initialize. + std::cout << "Initializing..." << std::endl; + auto engine = wasm::Engine::make(); + + std::cout << "Live count " << live_count << std::endl; + std::cout << "Creating store 1..." << std::endl; + auto store1 = wasm::Store::make(engine.get()); + + std::cout << "Running in store 1..." << std::endl; + run_in_store(store1.get()); + std::cout << "Live count " << live_count << std::endl; + + { + std::cout << "Creating store 2..." << std::endl; + auto store2 = wasm::Store::make(engine.get()); + + std::cout << "Running in store 2..." << std::endl; + run_in_store(store2.get()); + std::cout << "Live count " << live_count << std::endl; + + std::cout << "Deleting store 2..." << std::endl; + std::cout << "Live count " << live_count << std::endl; + } + + std::cout << "Running in store 1..." << std::endl; + run_in_store(store1.get()); + std::cout << "Live count " << live_count << std::endl; + + std::cout << "Deleting store 1..." << std::endl; +} + + int main(int argc, const char* argv[]) { run(); + std::cout << "Live count " << live_count << std::endl; + assert(live_count == 0); std::cout << "Done." << std::endl; return 0; }