Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(94)

Unified Diff: src/managed.h

Issue 2676513008: [wasm] Managed<T> ensures T's lifetime does not leak past Isolate's (Closed)
Patch Set: moved cctest Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« src/isolate.cc ('K') | « src/isolate.cc ('k') | src/v8.gyp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/managed.h
diff --git a/src/wasm/managed.h b/src/managed.h
similarity index 50%
rename from src/wasm/managed.h
rename to src/managed.h
index 5c9248ee3fd0331591cad21b8fee0ab44b208b2e..22da33866e2870edfe11f8ed73e5be8708332a23 100644
--- a/src/wasm/managed.h
+++ b/src/managed.h
@@ -12,13 +12,19 @@
namespace v8 {
namespace internal {
-// An object that wraps a pointer to a C++ object and optionally deletes it
-// when the managed wrapper object is garbage collected.
+// An object that wraps a pointer to a C++ object and manages its lifetime.
+// The C++ object will be deleted when the managed wrapper object is
+// garbage collected, or, last resort, if the isolate is torn down before GC,
+// as part of Isolate::Dispose().
+// Managed<CppType> may be used polymorphically as Foreign, where the held
+// address is typed as CppType**. The double indirection is due to the
+// use, by Managed, of Isolate::ManagedLifeline, which has a CppType* first
+// field.
template <class CppType>
class Managed : public Foreign {
public:
V8_INLINE CppType* get() {
- return reinterpret_cast<CppType*>(foreign_address());
+ return *(reinterpret_cast<CppType**>(foreign_address()));
}
static Managed<CppType>* cast(Object* obj) {
@@ -26,13 +32,12 @@ class Managed : public Foreign {
return reinterpret_cast<Managed<CppType>*>(obj);
}
- static Handle<Managed<CppType>> New(Isolate* isolate, CppType* ptr,
- bool delete_on_gc = true) {
+ static Handle<Managed<CppType>> New(Isolate* isolate, CppType* ptr) {
+ Isolate::ManagedLifeline* node = isolate->RegisterForReleaseAtTeardown(
+ ptr, Managed<CppType>::NativeDelete);
Handle<Managed<CppType>> handle = Handle<Managed<CppType>>::cast(
- isolate->factory()->NewForeign(reinterpret_cast<Address>(ptr)));
- if (delete_on_gc) {
- RegisterWeakCallbackForDelete(isolate, handle);
- }
+ isolate->factory()->NewForeign(reinterpret_cast<Address>(node)));
+ RegisterWeakCallbackForDelete(isolate, handle);
return handle;
}
@@ -41,16 +46,32 @@ class Managed : public Foreign {
Handle<Managed<CppType>> handle) {
Handle<Object> global_handle = isolate->global_handles()->Create(*handle);
GlobalHandles::MakeWeak(global_handle.location(), global_handle.location(),
- &Managed<CppType>::Delete,
+ &Managed<CppType>::GCDelete,
v8::WeakCallbackType::kFinalizer);
}
- static void Delete(const v8::WeakCallbackInfo<void>& data) {
+
+ static void GCDelete(const v8::WeakCallbackInfo<void>& data) {
Managed<CppType>** p =
reinterpret_cast<Managed<CppType>**>(data.GetParameter());
- delete (*p)->get();
- (*p)->set_foreign_address(0);
+
+ Isolate::ManagedLifeline* lifeline = (*p)->GetLifeline();
+
+ Isolate* isolate = reinterpret_cast<Isolate*>(data.GetIsolate());
+ lifeline->Dispose();
+ isolate->UnregisterFromReleaseAtTeardown(&lifeline);
+
+ (*p)->set_foreign_address(static_cast<Address>(nullptr));
GlobalHandles::Destroy(reinterpret_cast<Object**>(p));
}
+
+ static void NativeDelete(void* value) {
+ CppType* typed_value = reinterpret_cast<CppType*>(value);
+ delete typed_value;
+ }
+
+ Isolate::ManagedLifeline* GetLifeline() {
+ return reinterpret_cast<Isolate::ManagedLifeline*>(foreign_address());
+ }
};
} // namespace internal
} // namespace v8
« src/isolate.cc ('K') | « src/isolate.cc ('k') | src/v8.gyp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698