diff --git a/ChangeLog.md b/ChangeLog.md index 7bc619cc29ba..f29c3c867384 100644 --- a/ChangeLog.md +++ b/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 ----------------- diff --git a/src/embind/embind.js b/src/embind/embind.js index 9f4789565132..45d9c0d6fa55 100644 --- a/src/embind/embind.js +++ b/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, }); }, diff --git a/src/embind/emval.js b/src/embind/emval.js index 90fb44ebb0b0..174834aa64a3 100644 --- a/src/embind/emval.js +++ b/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); diff --git a/test/code_size/embind_val_wasm.json b/test/code_size/embind_val_wasm.json index b84b10f6bcb7..116edabb12d4 100644 --- a/test/code_size/embind_val_wasm.json +++ b/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 } diff --git a/test/embind/embind.test.js b/test/embind/embind.test.js index 7253e639e2c1..8a5cdd18eafa 100644 --- a/test/embind/embind.test.js +++ b/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()); diff --git a/test/embind/embind_test.cpp b/test/embind/embind_test.cpp index 6f84290001ff..aa83e935dc2a 100644 --- a/test/embind/embind_test.cpp +++ b/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"); + 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);