[emval] Move lifecycle mangement from JS to C++.

src/embind/embind.js

@@ -9,7 +9,7 @@
 /*global _malloc, _free, _memcpy*/
 /*global FUNCTION_TABLE, HEAP8, HEAPU8, HEAP16, HEAPU16, HEAP32, HEAPU32, HEAPF32, HEAPF64*/
 /*global readLatin1String*/
-/*global Emval, emval_handle_array, __emval_decref*/
+/*global Emval, __emval_free*/
 /*jslint sub:true*/ /* The symbols 'fromWireType' and 'toWireType' must be accessed via array notation to be closure-safe since craftInvokerFunction crafts functions as strings that can't be closured. */
 
 // -- jshint doesn't understand library syntax, so we need to specifically tell it about the symbols we define
@@ -41,12 +41,12 @@ var LibraryEmbind = {
   // If register_type is used, emval will be registered multiple times for
   // different type id's, but only a single type object is needed on the JS side
   // for all of them. Store the type for reuse.
-  $EmValType__deps: ['_emval_decref', '$Emval', '$readPointer', '$GenericWireTypeSize'],
+  $EmValType__deps: ['_emval_free', '$Emval', '$readPointer', '$GenericWireTypeSize'],
   $EmValType: `{
     name: 'emscripten::val',
     'fromWireType': (handle) => {
       var rv = Emval.toValue(handle);
-      __emval_decref(handle);
+      __emval_free(handle);
       return rv;
     },
     'toWireType': (destructors, value) => Emval.toHandle(value),

src/embind/emval.js

@@ -12,43 +12,35 @@
 /*jslint sub:true*/ /* The symbols 'fromWireType' and 'toWireType' must be accessed via array notation to be closure-safe since craftInvokerFunction crafts functions as strings that can't be closured. */
 
 // -- jshint doesn't understand library syntax, so we need to mark the symbols exposed here
-/*global getStringOrSymbol, emval_freelist, emval_handles, Emval, __emval_unregister, count_emval_handles, emval_symbols, __emval_decref*/
+/*global getStringOrSymbol, emval_handles, Emval, __emval_unregister, count_emval_handles, emval_symbols, __emval_free*/
 /*global emval_addMethodCaller, emval_methodCallers, addToLibrary, global, emval_lookupTypes, makeLegalFunctionName*/
 /*global emval_get_global*/
 
-// Number of handles reserved for non-use (0) or common values w/o refcount.
+// Number of handles reserved for non-use (0) or common values never freed.
 {{{ 
-  globalThis.EMVAL_RESERVED_HANDLES = 5;
-  globalThis.EMVAL_LAST_RESERVED_HANDLE = globalThis.EMVAL_RESERVED_HANDLES * 2 - 1;
+  globalThis.EMVAL_RESERVED_HANDLES = 4;
   null;
 }}}
 var LibraryEmVal = {
-  // Stack of handles available for reuse.
-  $emval_freelist: [],
-  // Array of alternating pairs (value, refcount).
-  $emval_handles: [],
+  $emval_handles__deps: ['$HandleAllocator'],
+  $emval_handles: "new HandleAllocator();",
   $emval_symbols: {}, // address -> string
 
   $init_emval__deps: ['$count_emval_handles', '$emval_handles'],
   $init_emval__postset: 'init_emval();',
   $init_emval: () => {
-    // reserve 0 and some special values. These never get de-allocated.
-    emval_handles.push(
-      0, 1,
-      undefined, 1,
-      null, 1,
-      true, 1,
-      false, 1,
-    );
+    // reserve some special values. These never get de-allocated.
+    // The HandleAllocator takes care of reserving zero.
+    emval_handles.allocated.push(undefined, null, true, false);
   #if ASSERTIONS
-    assert(emval_handles.length === {{{ EMVAL_RESERVED_HANDLES }}} * 2);
+    assert(emval_handles.count() === {{{ EMVAL_RESERVED_HANDLES }}});
   #endif
     Module['count_emval_handles'] = count_emval_handles;
   },
 
-  $count_emval_handles__deps: ['$emval_freelist', '$emval_handles'],
+  $count_emval_handles__deps: ['$emval_handles'],
   $count_emval_handles: () => {
-    return emval_handles.length / 2 - {{{ EMVAL_RESERVED_HANDLES }}} - emval_freelist.length;
+    return emval_handles.count() - {{{ EMVAL_RESERVED_HANDLES }}};
   },
 
   _emval_register_symbol__deps: ['$emval_symbols', '$readLatin1String'],
@@ -65,58 +57,43 @@ var LibraryEmVal = {
     return symbol;
   },
 
-  $Emval__deps: ['$emval_freelist', '$emval_handles', '$throwBindingError', '$init_emval'],
+  $Emval__deps: ['$emval_handles', '$throwBindingError', '$init_emval'],
   $Emval: {
     toValue: (handle) => {
       if (!handle) {
           throwBindingError('Cannot use deleted val. handle = ' + handle);
       }
-  #if ASSERTIONS
-      // handle 2 is supposed to be `undefined`.
-      assert(handle === 2 || emval_handles[handle] !== undefined && handle % 2 === 0, `invalid handle: ${handle}`);
-  #endif
-      return emval_handles[handle];
+      return emval_handles.get(handle);
     },
 
     toHandle: (value) => {
       switch (value) {
-        case undefined: return 2;
-        case null: return 4;
-        case true: return 6;
-        case false: return 8;
+        case undefined: return 1;
+        case null: return 2;
+        case true: return 3;
+        case false: return 4;
         default:{
-          const handle = emval_freelist.pop() || emval_handles.length;
-          emval_handles[handle] = value;
-          emval_handles[handle + 1] = 1;
-          return handle;
+          return emval_handles.allocate(value);
         }
       }
     }
   },
 
-  _emval_incref__deps: ['$emval_handles'],
-  _emval_incref: (handle) => {
-    if (handle > {{{ EMVAL_LAST_RESERVED_HANDLE }}}) {
-      emval_handles[handle + 1] += 1;
-    }
-  },
+  _emval_clone__deps: ['$emval_handles'],
+  _emval_clone: (handle) => Emval.toHandle(emval_handles.get(handle)),
 
-  _emval_decref__deps: ['$emval_freelist', '$emval_handles'],
-  _emval_decref: (handle) => {
-    if (handle > {{{ EMVAL_LAST_RESERVED_HANDLE }}} && 0 === --emval_handles[handle + 1]) {
-  #if ASSERTIONS
-      assert(emval_handles[handle] !== undefined, `Decref for unallocated handle.`);
-  #endif
-      emval_handles[handle] = undefined;
-      emval_freelist.push(handle);
+  _emval_free__deps: ['$emval_handles'],
+  _emval_free: (handle) => {
+    if (handle > {{{ EMVAL_RESERVED_HANDLES }}}) {
+      emval_handles.free(handle);
     }
   },
 
-  _emval_run_destructors__deps: ['_emval_decref', '$Emval', '$runDestructors'],
+  _emval_run_destructors__deps: ['_emval_free', '$Emval', '$runDestructors'],
   _emval_run_destructors: (handle) => {
     var destructors = Emval.toValue(handle);
     runDestructors(destructors);
-    __emval_decref(handle);
+    __emval_free(handle);
   },
 
   _emval_new_array__deps: ['$Emval'],

src/library.js

@@ -3556,18 +3556,19 @@ addToLibrary({
     constructor() {
       // TODO(sbc): Use class fields once we allow/enable es2022 in
       // JavaScript input to acorn and closure.
+      // Use this allocator object as a placeholder for invalid entries.
       // Reserve slot 0 so that 0 is always an invalid handle
-      this.allocated = [undefined];
+      this.allocated = [this];
       this.freelist = [];
     }
     get(id) {
 #if ASSERTIONS
-      assert(this.allocated[id] !== undefined, `invalid handle: ${id}`);
+      assert(this.allocated[id] !== this, `invalid handle: ${id}`);
 #endif
       return this.allocated[id];
     };
     has(id) {
-      return this.allocated[id] !== undefined;
+      return this.allocated[id] !== this;
     };
     allocate(handle) {
       var id = this.freelist.pop() || this.allocated.length;
@@ -3576,13 +3577,15 @@ addToLibrary({
     };
     free(id) {
 #if ASSERTIONS
-      assert(this.allocated[id] !== undefined);
+      assert(this.allocated[id] !== this);
 #endif
-      // Set the slot to `undefined` rather than using `delete` here since
-      // apparently arrays with holes in them can be less efficient.
-      this.allocated[id] = undefined;
+      this.allocated[id] = this;
       this.freelist.push(id);
     };
+    count() {
+      // Handle zero is reserved and always allocated.
+      return this.allocated.length - this.freelist.length - 1;
+    }
   },
 
   $getNativeTypeSize__deps: ['$POINTER_SIZE'],

src/library_sigs.js

@@ -342,18 +342,18 @@ sigs = {
   _emval_await__sig: 'pp',
   _emval_call__sig: 'dpppp',
   _emval_call_method__sig: 'dppppp',
+  _emval_clone__sig: 'pp',
   _emval_coro_make_promise__sig: 'ppp',
   _emval_coro_suspend__sig: 'vpp',
-  _emval_decref__sig: 'vp',
   _emval_delete__sig: 'ipp',
   _emval_equals__sig: 'ipp',
+  _emval_free__sig: 'vp',
   _emval_get_global__sig: 'pp',
   _emval_get_method_caller__sig: 'pipi',
   _emval_get_module_property__sig: 'pp',
   _emval_get_property__sig: 'ppp',
   _emval_greater_than__sig: 'ipp',
   _emval_in__sig: 'ipp',
-  _emval_incref__sig: 'vp',
   _emval_instanceof__sig: 'ipp',
   _emval_is_number__sig: 'ip',
   _emval_is_string__sig: 'ip',

system/include/emscripten/val.h

@@ -46,19 +46,20 @@ extern "C" {
 void _emval_register_symbol(const char*);
 
 enum {
-  _EMVAL_UNDEFINED = 2,
-  _EMVAL_NULL = 4,
-  _EMVAL_TRUE = 6,
-  _EMVAL_FALSE = 8
+  _EMVAL_UNDEFINED = 1,
+  _EMVAL_NULL = 2,
+  _EMVAL_TRUE = 3,
+  _EMVAL_FALSE = 4,
+  _EMVAL_NUM_RESERVED_HANDLES = 5,
 };
 
 typedef struct _EM_DESTRUCTORS* EM_DESTRUCTORS;
 typedef struct _EM_METHOD_CALLER* EM_METHOD_CALLER;
 typedef double EM_GENERIC_WIRE_TYPE;
 typedef const void* EM_VAR_ARGS;
 
-void _emval_incref(EM_VAL value);
-void _emval_decref(EM_VAL value);
+EM_VAL _emval_clone(EM_VAL value);
+void _emval_free(EM_VAL value);
 
 void _emval_run_destructors(EM_DESTRUCTORS handle);
 
@@ -376,21 +377,37 @@ class val {
       : val(internal::_emval_new_cstring(v))
   {}
 
-  // Note: unlike other constructors, this doesn't use as_handle() because
-  // it just moves a value and doesn't need to go via incref/decref.
-  // This means it's safe to move values across threads - an error will
-  // only arise if you access or free it from the wrong thread later.
-  val(val&& v) : handle(v.handle), thread(v.thread) {
+  // Move constructor takes over the other item's place in the list for this handle.
+  val(val&& v) : val(v.handle, v.thread) {
+    if (v.next != &v) {
+      next = v.next;
+      prev = v.prev;
+      next->prev = prev->next = this;
+    }
     v.handle = 0;
+    v.next = v.prev = &v;
   }
 
-  val(const val& v) : val(v.as_handle()) {
-    internal::_emval_incref(handle);
+  // Copy constructor inserts this new entry into the list for this handle.
+  val(const val& v) : val(v.handle, v.thread) {
+    next = v.next;
+    prev = &v;
+    prev->next = this;
+    next->prev = this;
   }
 
   ~val() {
     if (handle) {
-      internal::_emval_decref(as_handle());
+      if (this == next) {
+        // The was the only entry in the list, free the value.
+        if (handle >= reinterpret_cast<EM_VAL>(internal::_EMVAL_NUM_RESERVED_HANDLES)) {
+          internal::_emval_free(as_handle());
+        }
+      } else {
+        // More references exist, remove this entry from the list.
+        next->prev = prev;
+        prev->next = next;
+      }
       handle = 0;
     }
   }
@@ -625,10 +642,11 @@ class val {
 #endif
 
 private:
-  // takes ownership, assumes handle already incref'd and lives on the same thread
-  explicit val(EM_VAL handle)
-      : handle(handle), thread(pthread_self())
-  {}
+  // takes ownership, assumes handle lives on the same thread with no other C++ references
+  explicit val(EM_VAL handle) : val(handle, pthread_self()) {}
+
+  val(EM_VAL handle, pthread_t thread)
+      : handle(handle), next(this), prev(this), thread(pthread_self()) {}
 
   template<typename WrapperType>
   friend val internal::wrapped_extend(const std::string& , const val& );
@@ -657,8 +675,12 @@ class val {
     return v;
   }
 
-  pthread_t thread;
   EM_VAL handle;
+  // Circular doubly-linked list of all references to this handle, may
+  // be mutated by copy constructor of new instance inserting itself.
+  const val mutable* next;
+  const val mutable* prev;
+  pthread_t thread;
 
   friend struct internal::BindingType<val>;
 };
@@ -787,9 +809,8 @@ struct BindingType<T, typename std::enable_if<std::is_base_of<val, T>::value &&
                                               !std::is_const<T>::value>::type> {
   typedef EM_VAL WireType;
   static WireType toWireType(const val& v) {
-    EM_VAL handle = v.as_handle();
-    _emval_incref(handle);
-    return handle;
+    // Allocate a new handle that the JS fromWireType will free.
+    return _emval_clone(v.as_handle());
   }
   static T fromWireType(WireType v) {
     return T(val::take_ownership(v));

test/code_size/embind_val_wasm.json

@@ -1,10 +1,10 @@
 {
   "a.html": 673,
   "a.html.gz": 431,
-  "a.js": 7080,
-  "a.js.gz": 2999,
-  "a.wasm": 11433,
-  "a.wasm.gz": 5725,
-  "total": 19186,
-  "total_gz": 9155
+  "a.js": 7214,
+  "a.js.gz": 3036,
+  "a.wasm": 11498,
+  "a.wasm.gz": 5766,
+  "total": 19385,
+  "total_gz": 9233
 }

test/core/test_promise.c

@@ -503,7 +503,7 @@ static em_promise_result_t finish(void** result, void* data, void* value) {
 
   // We should not have leaked any handles.
   EM_ASM({
-    promiseMap.allocated.forEach((p) => assert(typeof p === "undefined", "non-destroyed handle"));
+    assert(promiseMap.count() === 0, "non-destroyed handle");
   });
 
   // Cannot exit directly in a promise callback, since it would end up rejecting