Update Embind+GC integration, vol. 2. (#15874)

An updated version of #15327 (that was reverted at #15460). Finalization will
only affect smart ptrs now, as discussed there. I also added some notes on
this to documentation.

Author: mmarczell-graphisoft

site/source/docs/porting/connecting_cpp_and_javascript/embind.rst

@@ -202,12 +202,14 @@ to enable the closure compiler.
 Memory management
 =================
 
-JavaScript, specifically ECMA-262 Edition 5.1, does not support `finalizers`_
-or weak references with callbacks. Therefore there is no way for Emscripten
-to automatically call the destructors on C++ objects.
+JavaScript only gained support for `finalizers`_ in ECMAScript 2021, or ECMA-262
+Edition 12. The new API is called `FinalizationRegistry`_ and it still does not
+offer any guarantees that the provided finalization callback will be called.
+Embind uses this for cleanup if available, but only for smart pointers,
+and only as a last resort.
 
-.. warning:: JavaScript code must explicitly delete any C++ object handles
-   it has received, or the Emscripten heap will grow indefinitely.
+.. warning:: It is strongly recommended that JavaScript code explicitly deletes
+    any C++ object handles it has received.
 
 The :js:func:`delete()` JavaScript method is provided to manually signal that
 a C++ object is no longer needed and can be deleted:
@@ -1022,6 +1024,7 @@ real-world applications has proved to be more than acceptable.
 .. _Connecting C++ and JavaScript on the Web with Embind: http://chadaustin.me/2014/09/connecting-c-and-javascript-on-the-web-with-embind/
 .. _Boost.Python: http://www.boost.org/doc/libs/1_56_0/libs/python/doc/
 .. _finalizers: http://en.wikipedia.org/wiki/Finalizer
+.. _FinalizationRegistry: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/FinalizationRegistry
 .. _Reference Counting: https://en.wikipedia.org/wiki/Reference_counting
 .. _Boost.Python-like raw pointer policies: https://wiki.python.org/moin/boost.python/CallPolicy
 .. _Backbone.js: http://backbonejs.org/#Model-extend

src/embind/embind.js

@@ -17,7 +17,7 @@
 /*global typeDependencies, flushPendingDeletes, getTypeName, getBasestPointer, throwBindingError, UnboundTypeError, _embind_repr, registeredInstances, registeredTypes, getShiftFromSize*/
 /*global ensureOverloadTable, embind__requireFunction, awaitingDependencies, makeLegalFunctionName, embind_charCodes:true, registerType, createNamedFunction, RegisteredPointer, throwInternalError*/
 /*global simpleReadValueFromPointer, floatReadValueFromPointer, integerReadValueFromPointer, enumReadValueFromPointer, replacePublicSymbol, craftInvokerFunction, tupleRegistrations*/
-/*global finalizationGroup, attachFinalizer, detachFinalizer, releaseClassHandle, runDestructor*/
+/*global finalizationRegistry, attachFinalizer, detachFinalizer, releaseClassHandle, runDestructor*/
 /*global ClassHandle, makeClassHandle, structRegistrations, whenDependentTypesAreResolved, BindingError, deletionQueue, delayFunction:true, upcastPointer*/
 /*global exposePublicSymbol, heap32VectorToArray, new_, RegisteredPointer_getPointee, RegisteredPointer_destructor, RegisteredPointer_deleteObject, char_0, char_9*/
 /*global getInheritedInstanceCount, getLiveInheritedInstances, setDelayFunction, InternalError, runDestructors*/
@@ -1734,39 +1734,53 @@ var LibraryEmbind = {
     }
   },
 
-  $finalizationGroup: false,
+  $finalizationRegistry: false,
 
-  $detachFinalizer_deps: ['$finalizationGroup'],
+  $detachFinalizer_deps: ['$finalizationRegistry'],
   $detachFinalizer: function(handle) {},
 
-  $attachFinalizer__deps: ['$finalizationGroup', '$detachFinalizer',
-                           '$releaseClassHandle'],
+  $attachFinalizer__deps: ['$finalizationRegistry', '$detachFinalizer',
+                           '$releaseClassHandle', '$RegisteredPointer_fromWireType'],
   $attachFinalizer: function(handle) {
-    if ('undefined' === typeof FinalizationGroup) {
+    if ('undefined' === typeof FinalizationRegistry) {
         attachFinalizer = (handle) => handle;
         return handle;
     }
-    // If the running environment has a FinalizationGroup (see
+    // If the running environment has a FinalizationRegistry (see
     // https://github.com/tc39/proposal-weakrefs), then attach finalizers
-    // for class handles.  We check for the presence of FinalizationGroup
+    // for class handles.  We check for the presence of FinalizationRegistry
     // at run-time, not build-time.
-    finalizationGroup = new FinalizationGroup(function (iter) {
-        for (var result = iter.next(); !result.done; result = iter.next()) {
-            var $$ = result.value;
-            if (!$$.ptr) {
-                console.warn('object already deleted: ' + $$.ptr);
-            } else {
-                releaseClassHandle($$);
-            }
-        }
+    finalizationRegistry = new FinalizationRegistry((info) => {
+#if ASSERTIONS
+        console.warn(info.leakWarning.stack.replace(/^Error: /, ''));
+#endif
+        releaseClassHandle(info.$$);
     });
     attachFinalizer = (handle) => {
-        finalizationGroup.register(handle, handle.$$, handle.$$);
+        var $$ = handle.$$;
+        var hasSmartPtr = !!$$.smartPtr;
+        if (hasSmartPtr) {
+            // We should not call the destructor on raw pointers in case other code expects the pointee to live
+            var info = { $$: $$ };
+#if ASSERTIONS
+            // Create a warning as an Error instance in advance so that we can store
+            // the current stacktrace and point to it when / if a leak is detected.
+            // This is more useful than the empty stacktrace of `FinalizationRegistry`
+            // callback.
+            var cls = $$.ptrType.registeredClass;
+            info.leakWarning = new Error("Embind found a leaked C++ instance " + cls.name + " <0x" + $$.ptr.toString(16) + ">.\n" +
+            "We'll free it automatically in this case, but this functionality is not reliable across various environments.\n" +
+            "Make sure to invoke .delete() manually once you're done with the instance instead.\n" +
+            "Originally allocated"); // `.stack` will add "at ..." after this sentence
+            if ('captureStackTrace' in Error) {
+                Error.captureStackTrace(info.leakWarning, RegisteredPointer_fromWireType);
+            }
+ #endif
+            finalizationRegistry.register(handle, info, handle);
+        }
         return handle;
     };
-    detachFinalizer = (handle) => {
-        finalizationGroup.unregister(handle.$$);
-    };
+    detachFinalizer = (handle) => finalizationRegistry.unregister(handle);
     return attachFinalizer(handle);
   },
 

tests/embind/test_finalization.cpp

@@ -0,0 +1,27 @@
+#include <emscripten/bind.h>
+#include <iostream>
+#include <memory>
+
+class Foo {
+  std::string mName;
+
+public:
+  Foo(std::string name) : mName(name) {}
+  ~Foo() { std::cout << mName << " destructed" << std::endl; }
+};
+
+std::shared_ptr<Foo> foo() {
+  return std::make_shared<Foo>("Constructed from C++");
+}
+
+Foo* pFoo() { return new Foo("Foo*"); }
+
+using namespace emscripten;
+
+EMSCRIPTEN_BINDINGS(Marci) {
+  class_<Foo>("Foo").smart_ptr_constructor<std::shared_ptr<Foo>>(
+    "Foo", &std::make_shared<Foo, std::string>);
+
+  function("foo", foo);
+  function("pFoo", pFoo, allow_raw_pointers());
+}

tests/embind/test_finalization.js

@@ -0,0 +1,7 @@
+Module.onRuntimeInitialized = () => {
+  const foo1 = new Module.Foo("Constructed from JS");
+  const foo2 = Module.foo();
+  const foo3 = Module.pFoo();
+}
+
+setTimeout(gc, 100);

tests/test_other.py

@@ -2427,6 +2427,19 @@ def test_embind(self):
       output = self.run_js('a.out.js')
       self.assertNotContained('FAIL', output)
 
+  def test_embind_finalization(self):
+    self.run_process(
+      [EMXX,
+       test_file('embind/test_finalization.cpp'),
+       '--post-js', test_file('embind/test_finalization.js'),
+       '--bind']
+    )
+    self.node_args += ['--expose-gc']
+    output = self.run_js('a.out.js', engine=config.NODE_JS)
+    self.assertContained('Constructed from C++ destructed', output)
+    self.assertContained('Constructed from JS destructed', output)
+    self.assertNotContained('Foo* destructed', output)
+
   def test_emconfig(self):
     output = self.run_process([emconfig, 'LLVM_ROOT'], stdout=PIPE).stdout.strip()
     self.assertEqual(output, config.LLVM_ROOT)