embind: Don't automatically destroy arguments passed into val calls.

ChangeLog.md

@@ -50,6 +50,9 @@ See docs/process.md for more on how version tagging works.
   `MIN_CHROME_VERSION` will now result in build-time error.  All of these
   browser versions are at least 8 years old now so the hope is that nobody
   is intending to target them today.  (#20924)
+- C++ objects passed into embind's val via constructors, methods, and call
+  function will not be automatically destroyed after the function call. This
+  makes the behavior consistent for invocations. 
 
 3.1.51 - 12/13/23
 -----------------

src/embind/embind.js

@@ -678,9 +678,6 @@ var LibraryEmbind = {
       'argPackAdvance': GenericWireTypeSize,
       'readValueFromPointer': simpleReadValueFromPointer,
       destructorFunction: null, // This type does not need a destructor
-
-      // TODO: do we need a deleteObject here?  write a test where
-      // emval is passed into JS via an interface
     });
   },
 
@@ -1311,11 +1308,6 @@ var LibraryEmbind = {
       },
       'argPackAdvance': GenericWireTypeSize,
       'readValueFromPointer': readPointer,
-      'deleteObject'(handle) {
-        if (handle !== null) {
-          handle['delete']();
-        }
-      },
       'fromWireType': RegisteredPointer_fromWireType,
     });
   },

src/embind/emval.js

@@ -335,9 +335,6 @@ var LibraryEmVal = {
         offset += types[i]['argPackAdvance'];
       }
       var rv = kind === /* CONSTRUCTOR */ 1 ? reflectConstruct(func, argN) : func.apply(obj, argN);
-      for (var i = 0; i < argCount; ++i) {
-        types[i].deleteObject?.(argN[i]);
-      }
       return emval_returnValue(retType, destructorsRef, rv);
     };
 #else
@@ -362,12 +359,6 @@ var LibraryEmVal = {
     var invoker = kind === /* CONSTRUCTOR */ 1 ? 'new func' : 'func.call';
     functionBody +=
       `  var rv = ${invoker}(${argsList.join(", ")});\n`;
-    for (var i = 0; i < argCount; ++i) {
-      if (types[i]['deleteObject']) {
-        functionBody +=
-          `  argType${i}.deleteObject(arg${i});\n`;
-      }
-    }
     if (!retType.isVoid) {
       params.push("emval_returnValue");
       args.push(emval_returnValue);

test/code_size/embind_val_wasm.json

@@ -1,10 +1,10 @@
 {
   "a.html": 673,
   "a.html.gz": 431,
-  "a.js": 7426,
-  "a.js.gz": 3122,
+  "a.js": 7395,
+  "a.js.gz": 3109,
   "a.wasm": 11458,
   "a.wasm.gz": 5733,
-  "total": 19557,
-  "total_gz": 9286
+  "total": 19526,
+  "total_gz": 9273
 }

test/embind/embind.test.js

@@ -707,6 +707,10 @@ module({
             cm.emval_test_take_and_return_std_string_const_ref("foobar");
         });
 
+        test("val callback arguments are not destroyed", function() {
+            cm.emval_test_callback_arg_lifetime(function() {});
+        });
+
         test("can get global", function(){
             /*jshint evil:true*/
             assert.equal((new Function("return this;"))(), cm.embind_test_getglobal());

test/embind/embind_test.cpp

@@ -127,6 +127,22 @@ unsigned emval_test_sum(val v) {
     return rv;
 }
 
+struct DestructorCounter {
+    static int count;
+    ~DestructorCounter() {
+        count++;
+    };
+};
+
+int DestructorCounter::count = 0;
+
+void emval_test_callback_arg_lifetime(val callback) {
+    DestructorCounter dc;
+    int destructorCount = DestructorCounter::count;
+    callback(dc);
+    assert(destructorCount == DestructorCounter::count);
+}
+
 std::string get_non_ascii_string(bool embindStdStringUTF8Support) {
     if(embindStdStringUTF8Support) {
         //ASCII
@@ -1828,6 +1844,9 @@ EMSCRIPTEN_BINDINGS(tests) {
     function("const_ref_adder", &const_ref_adder);
     function("emval_test_sum", &emval_test_sum);
 
+    class_<DestructorCounter>("DestructorCounter");
+    function("emval_test_callback_arg_lifetime", &emval_test_callback_arg_lifetime);
+
     function("get_non_ascii_string", &get_non_ascii_string);
     function("get_non_ascii_wstring", &get_non_ascii_wstring);
     function("get_literal_wstring", &get_literal_wstring);