Bug 1542184 - Use PersistentRooted for rooting vectors of GC things from rust code r=fitzgen?
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 10 May 2019 17:36:34 +0000
changeset 532403 4b8f9814a1ea6f412d1f02c2701096b048cd0707
parent 532402 430b79a1444fd6935ea68abfbdb7a53d8e572b78
child 532404 a0d20ab6be8b779fdd96a14f0cfd07ffe73c8e3d
push id11268
push usercsabou@mozilla.com
push dateTue, 14 May 2019 15:24:22 +0000
treeherdermozilla-beta@5fb7fcd568d6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs1542184
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1542184 - Use PersistentRooted for rooting vectors of GC things from rust code r=fitzgen? This replaces the use of heap-alloced Rooted with PersistentRooted which is safe wrt destruction order. I had to add PersistentRooted and StackGCVector to OPAQUE_TYPES to make this work... I'm not really sure what this does. Differential Revision: https://phabricator.services.mozilla.com/D30668
js/public/GCVector.h
js/public/TypeDecls.h
js/rust/build.rs
js/rust/src/glue.rs
js/rust/src/jsglue.cpp
js/rust/src/rust.rs
--- a/js/public/GCVector.h
+++ b/js/public/GCVector.h
@@ -299,11 +299,22 @@ template <typename T>
 class RootedVector : public Rooted<StackGCVector<T>> {
   using Vec = StackGCVector<T>;
   using Base = Rooted<Vec>;
 
  public:
   explicit RootedVector(JSContext* cx) : Base(cx, Vec(cx)) {}
 };
 
+// For use in rust code, an analog to RootedVector that doesn't require
+// instances to be destroyed in LIFO order.
+template <typename T>
+class PersistentRootedVector : public PersistentRooted<StackGCVector<T>> {
+  using Vec = StackGCVector<T>;
+  using Base = PersistentRooted<Vec>;
+
+ public:
+  explicit PersistentRootedVector(JSContext* cx) : Base(cx, Vec(cx)) {}
+};
+
 }  // namespace JS
 
 #endif  // js_GCVector_h
--- a/js/public/TypeDecls.h
+++ b/js/public/TypeDecls.h
@@ -57,16 +57,18 @@ class Handle;
 template <typename T>
 class MutableHandle;
 template <typename T>
 class Rooted;
 template <typename T>
 class PersistentRooted;
 template <typename T>
 class RootedVector;
+template <typename T>
+class PersistentRootedVector;
 template <typename T, typename AllocPolicy = js::TempAllocPolicy>
 class StackGCVector;
 
 typedef Handle<JSFunction*> HandleFunction;
 typedef Handle<PropertyKey> HandleId;
 typedef Handle<JSObject*> HandleObject;
 typedef Handle<JSScript*> HandleScript;
 typedef Handle<JSString*> HandleString;
@@ -106,16 +108,19 @@ typedef PersistentRooted<JSFunction*> Pe
 typedef PersistentRooted<PropertyKey> PersistentRootedId;
 typedef PersistentRooted<JSObject*> PersistentRootedObject;
 typedef PersistentRooted<JSScript*> PersistentRootedScript;
 typedef PersistentRooted<JSString*> PersistentRootedString;
 typedef PersistentRooted<JS::Symbol*> PersistentRootedSymbol;
 typedef PersistentRooted<JS::BigInt*> PersistentRootedBigInt;
 typedef PersistentRooted<Value> PersistentRootedValue;
 
+typedef PersistentRootedVector<PropertyKey> PersistentRootedIdVector;
+typedef PersistentRootedVector<JSObject*> PersistentRootedObjectVector;
+
 template <typename T>
 using HandleVector = Handle<StackGCVector<T>>;
 template <typename T>
 using MutableHandleVector = MutableHandle<StackGCVector<T>>;
 }  // namespace JS
 
 using jsid = JS::PropertyKey;
 
--- a/js/rust/build.rs
+++ b/js/rust/build.rs
@@ -147,17 +147,16 @@ const EXTRA_CLANG_FLAGS: &'static [&'sta
     "-fno-sized-deallocation",
     "-DRUST_BINDGEN",
 ];
 
 /// Types which we want to generate bindings for (and every other type they
 /// transitively use).
 const WHITELIST_TYPES: &'static [&'static str] = &[
     "JS::AutoCheckCannotGC",
-    "JS::RootedIdVector",
     "JS::CallArgs",
     "js::Class",
     "JS::RealmOptions",
     "JS::ContextOptions",
     "js::DOMCallbacks",
     "js::DOMProxyShadowsResult",
     "js::ESClass",
     "JS::ForOfIterator",
@@ -211,21 +210,22 @@ const WHITELIST_TYPES: &'static [&'stati
     "JS::Latin1Char",
     "JS::detail::MaybeWrapped",
     "JS::MutableHandle",
     "JS::MutableHandleObject",
     "JS::MutableHandleValue",
     "JS::NativeImpl",
     "js::ObjectOps",
     "JS::ObjectOpResult",
+    "JS::PersistentRootedIdVector",
+    "JS::PersistentRootedObjectVector",
     "JS::PromiseState",
     "JS::PropertyDescriptor",
     "JS::Rooted",
     "JS::RootedObject",
-    "JS::RootedObjectVector",
     "JS::RootedValue",
     "JS::RootingContext",
     "JS::RootKind",
     "js::Scalar::Type",
     "JS::ServoSizes",
     "js::shadow::Object",
     "js::shadow::ObjectGroup",
     "JS::SourceText",
@@ -471,20 +471,18 @@ const WHITELIST_FUNCTIONS: &'static [&'s
 ///
 /// These are types which are too tricky for bindgen to handle, and/or use C++
 /// features that don't have an equivalent in rust, such as partial template
 /// specialization.
 const OPAQUE_TYPES: &'static [&'static str] = &[
     "JS::ReadOnlyCompileOptions",
     "mozilla::BufferList",
     "mozilla::UniquePtr.*",
-    "JS::RootedVector",
-    "JS::Rooted<JS::Auto.*Vector.*>",
-    "JS::Auto.*Vector",
-    "JS::StackGCVector"
+    "JS::PersistentRooted",
+    "JS::StackGCVector",
 ];
 
 /// Types for which we should NEVER generate bindings, even if it is used within
 /// a type or function signature that we are generating bindings for.
 const BLACKLIST_TYPES: &'static [&'static str] = &[
     // We provide our own definition because we need to express trait bounds in
     // the definition of the struct to make our Drop implementation correct.
     "JS::Heap",
--- a/js/rust/src/glue.rs
+++ b/js/rust/src/glue.rs
@@ -252,24 +252,24 @@ extern "C" {
                                    -> *const JSErrorFormatString;
     pub fn IsProxyHandlerFamily(obj: *mut JSObject) -> u8;
     pub fn GetProxyHandlerExtra(obj: *mut JSObject) -> *const ::libc::c_void;
     pub fn GetProxyHandler(obj: *mut JSObject) -> *const ::libc::c_void;
     pub fn ReportError(aCx: *mut JSContext, aError: *const i8);
     pub fn IsWrapper(obj: *mut JSObject) -> bool;
     pub fn UnwrapObjectStatic(obj: *mut JSObject) -> *mut JSObject;
     pub fn UncheckedUnwrapObject(obj: *mut JSObject, stopAtOuter: u8) -> *mut JSObject;
-    pub fn CreateRootedIdVector(cx: *mut JSContext) -> *mut JS::RootedIdVector;
-    pub fn AppendToRootedIdVector(v: *mut JS::RootedIdVector, id: jsid) -> bool;
-    pub fn SliceRootedIdVector(v: *const JS::RootedIdVector, length: *mut usize) -> *const jsid;
-    pub fn DestroyRootedIdVector(v: *mut JS::RootedIdVector);
-    pub fn GetMutableHandleIdVector(v: *mut JS::RootedIdVector)-> JS::MutableHandleIdVector;
-    pub fn CreateRootedObjectVector(aCx: *mut JSContext) -> *mut JS::RootedObjectVector;
-    pub fn AppendToRootedObjectVector(v: *mut JS::RootedObjectVector, obj: *mut JSObject) -> bool;
-    pub fn DeleteRootedObjectVector(v: *mut JS::RootedObjectVector);
+    pub fn CreateRootedIdVector(cx: *mut JSContext) -> *mut JS::PersistentRootedIdVector;
+    pub fn AppendToRootedIdVector(v: *mut JS::PersistentRootedIdVector, id: jsid) -> bool;
+    pub fn SliceRootedIdVector(v: *const JS::PersistentRootedIdVector, length: *mut usize) -> *const jsid;
+    pub fn DestroyRootedIdVector(v: *mut JS::PersistentRootedIdVector);
+    pub fn GetMutableHandleIdVector(v: *mut JS::PersistentRootedIdVector)-> JS::MutableHandleIdVector;
+    pub fn CreateRootedObjectVector(aCx: *mut JSContext) -> *mut JS::PersistentRootedObjectVector;
+    pub fn AppendToRootedObjectVector(v: *mut JS::PersistentRootedObjectVector, obj: *mut JSObject) -> bool;
+    pub fn DeleteRootedObjectVector(v: *mut JS::PersistentRootedObjectVector);
     pub fn CollectServoSizes(rt: *mut JSRuntime, sizes: *mut JS::ServoSizes) -> bool;
     pub fn CallIdTracer(trc: *mut JSTracer, idp: *mut Heap<jsid>, name: *const ::libc::c_char);
     pub fn CallValueTracer(trc: *mut JSTracer,
                            valuep: *mut Heap<JS::Value>,
                            name: *const ::libc::c_char);
     pub fn CallObjectTracer(trc: *mut JSTracer,
                             objp: *mut Heap<*mut JSObject>,
                             name: *const ::libc::c_char);
--- a/js/rust/src/jsglue.cpp
+++ b/js/rust/src/jsglue.cpp
@@ -533,45 +533,51 @@ bool IsWrapper(JSObject* obj) { return j
 JSObject* UnwrapObjectStatic(JSObject* obj) {
   return js::CheckedUnwrapStatic(obj);
 }
 
 JSObject* UncheckedUnwrapObject(JSObject* obj, bool stopAtOuter) {
   return js::UncheckedUnwrap(obj, stopAtOuter);
 }
 
-JS::RootedIdVector* CreateRootedIdVector(JSContext* cx) {
-  return new JS::RootedIdVector(cx);
+JS::PersistentRootedIdVector* CreateRootedIdVector(JSContext* cx) {
+  return new JS::PersistentRootedIdVector(cx);
 }
 
-bool AppendToRootedIdVector(JS::RootedIdVector* v, jsid id) {
+bool AppendToRootedIdVector(JS::PersistentRootedIdVector* v, jsid id) {
   return v->append(id);
 }
 
-const jsid* SliceRootedIdVector(const JS::RootedIdVector* v, size_t* length) {
+const jsid* SliceRootedIdVector(const JS::PersistentRootedIdVector* v,
+                                size_t* length) {
   *length = v->length();
   return v->begin();
 }
 
-void DestroyRootedIdVector(JS::RootedIdVector* v) { delete v; }
+void DestroyRootedIdVector(JS::PersistentRootedIdVector* v) { delete v; }
 
-JS::MutableHandleIdVector GetMutableHandleIdVector(JS::RootedIdVector* v) {
+JS::MutableHandleIdVector GetMutableHandleIdVector(JS::PersistentRootedIdVector* v) {
   return JS::MutableHandleIdVector(v);
 }
 
-JS::RootedObjectVector* CreateRootedObjectVector(JSContext* aCx) {
-  JS::RootedObjectVector* vec = new JS::RootedObjectVector(aCx);
+JS::PersistentRootedObjectVector* CreateRootedObjectVector(
+    JSContext* aCx) {
+  JS::PersistentRootedObjectVector* vec =
+      new JS::PersistentRootedObjectVector(aCx);
   return vec;
 }
 
-bool AppendToRootedObjectVector(JS::RootedObjectVector* v, JSObject* obj) {
+bool AppendToRootedObjectVector(JS::PersistentRootedObjectVector* v,
+                                          JSObject* obj) {
   return v->append(obj);
 }
 
-void DeleteRootedObjectVector(JS::RootedObjectVector* v) { delete v; }
+void DeleteRootedObjectVector(JS::PersistentRootedObjectVector* v) {
+  delete v;
+}
 
 #if defined(__linux__)
 #  include <malloc.h>
 #elif defined(__APPLE__)
 #  include <malloc/malloc.h>
 #elif defined(__MINGW32__) || defined(__MINGW64__)
 // nothing needed here
 #elif defined(_MSC_VER)
--- a/js/rust/src/rust.rs
+++ b/js/rust/src/rust.rs
@@ -751,17 +751,17 @@ impl JSJitSetterCallArgs {
         self._base.handle()
     }
 }
 
 // ___________________________________________________________________________
 // Wrappers around things in jsglue.cpp
 
 pub struct RootedObjectVectorWrapper {
-    pub ptr: *mut JS::RootedObjectVector
+    pub ptr: *mut JS::PersistentRootedObjectVector
 }
 
 impl RootedObjectVectorWrapper {
     pub fn new(cx: *mut JSContext) -> RootedObjectVectorWrapper {
         RootedObjectVectorWrapper {
             ptr: unsafe {
                  CreateRootedObjectVector(cx)
             }
@@ -930,17 +930,17 @@ pub unsafe extern fn report_warning(_cx:
 impl JSNativeWrapper {
     fn is_zeroed(&self) -> bool {
         let JSNativeWrapper { op, info } = *self;
         op.is_none() && info.is_null()
     }
 }
 
 pub struct RootedIdVectorWrapper {
-    pub ptr: *mut JS::RootedIdVector
+    pub ptr: *mut JS::PersistentRootedIdVector
 }
 
 impl RootedIdVectorWrapper {
     pub fn new(cx: *mut JSContext) -> RootedIdVectorWrapper {
         RootedIdVectorWrapper {
             ptr: unsafe {
                 CreateRootedIdVector(cx)
             }