diff options
| -rw-r--r-- | src/embind/embind.js | 9 | ||||
| -rw-r--r-- | src/embind/emval.js | 9 | ||||
| -rw-r--r-- | tests/embind/embind.test.js | 47 | ||||
| -rw-r--r-- | tests/embind/embind_test.cpp | 31 |
4 files changed, 95 insertions, 1 deletions
diff --git a/src/embind/embind.js b/src/embind/embind.js index 9f3e6eff..124ea569 100644 --- a/src/embind/embind.js +++ b/src/embind/embind.js @@ -590,6 +590,9 @@ function __embind_register_emval(rawType, name) { 'argPackAdvance': 8, '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 }); } @@ -1195,6 +1198,12 @@ RegisteredPointer.prototype.destructor = function destructor(ptr) { RegisteredPointer.prototype['argPackAdvance'] = 8; RegisteredPointer.prototype['readValueFromPointer'] = simpleReadValueFromPointer; +RegisteredPointer.prototype['deleteObject'] = function deleteObject(handle) { + if (handle !== null) { + handle['delete'](); + } +}; + RegisteredPointer.prototype['fromWireType'] = function fromWireType(ptr) { // ptr is a raw pointer (or a raw smartpointer) diff --git a/src/embind/emval.js b/src/embind/emval.js index 2b49fc83..1661bc02 100644 --- a/src/embind/emval.js +++ b/src/embind/emval.js @@ -265,7 +265,14 @@ function __emval_get_method_caller(argCount, argTypes) { " args += argType" + i + ".argPackAdvance;\n"; } functionBody += - " var rv = handle[name](" + argsList + ");\n" + + " var rv = handle[name](" + argsList + ");\n"; + for (var i = 0; i < argCount - 1; ++i) { + if (types[i + 1]['deleteObject']) { + functionBody += + " argType" + i + ".deleteObject(arg" + i + ");\n"; + } + } + functionBody += " return retType.toWireType(destructors, rv);\n" + "};\n"; diff --git a/tests/embind/embind.test.js b/tests/embind/embind.test.js index ae421f27..a6b2e98c 100644 --- a/tests/embind/embind.test.js +++ b/tests/embind/embind.test.js @@ -1953,6 +1953,53 @@ module({ rv.delete(); cm.flushPendingDeletes(); }); + + test("method arguments with pointer ownership semantics are cleaned up after call", function() { + var parent = cm.AbstractClass; + var C = parent.extend("C", { + abstractMethod: function() { + }, + }); + var impl = new C; + cm.passShared(impl); + impl.delete(); + }); + + test("method arguments with pointer ownership semantics can be cloned", function() { + var parent = cm.AbstractClass; + var owned; + var C = parent.extend("C", { + abstractMethod: function() { + }, + passShared: function(p) { + owned = p.clone(); + } + }); + var impl = new C; + cm.passShared(impl); + impl.delete(); + + assert.equal("Derived", owned.getClassName()); + owned.delete(); + }); + + test("emscripten::val method arguments don't leak", function() { + var parent = cm.AbstractClass; + var got; + var C = parent.extend("C", { + abstractMethod: function() { + }, + passVal: function(g) { + got = g; + } + }); + var impl = new C; + var v = {}; + cm.passVal(impl, v); + impl.delete(); + + assert.equal(v, got); + }); }); BaseFixture.extend("registration order", function() { diff --git a/tests/embind/embind_test.cpp b/tests/embind/embind_test.cpp index eb534134..a0cb858d 100644 --- a/tests/embind/embind_test.cpp +++ b/tests/embind/embind_test.cpp @@ -1094,6 +1094,12 @@ public: std::string concreteMethod() const { return "concrete"; } + + virtual void passShared(const std::shared_ptr<Derived>&) { + } + + virtual void passVal(const val& v) { + } }; EMSCRIPTEN_SYMBOL(optionalMethod); @@ -1120,6 +1126,14 @@ public: void differentArguments(int i, double d, unsigned char f, double q, std::string s) { return call<void>("differentArguments", i, d, f, q, s); } + + virtual void passShared(const std::shared_ptr<Derived>& p) override { + return call<void>("passShared", p); + } + + virtual void passVal(const val& v) override { + return call<void>("passVal", v); + } }; class ConcreteClass : public AbstractClass { @@ -1196,6 +1210,15 @@ std::shared_ptr<PolySecondBase> passHeldAbstractClass(std::shared_ptr<HeldAbstra return p; } +void passShared(AbstractClass& ac) { + auto p = std::make_shared<Derived>(); + ac.passShared(p); +} + +void passVal(AbstractClass& ac, val v) { + return ac.passVal(v); +} + EMSCRIPTEN_BINDINGS(interface_tests) { class_<AbstractClass>("AbstractClass") .smart_ptr<std::shared_ptr<AbstractClass>>("shared_ptr<AbstractClass>") @@ -1209,6 +1232,12 @@ EMSCRIPTEN_BINDINGS(interface_tests) { } )) .function("concreteMethod", &AbstractClass::concreteMethod) + .function("passShared", select_overload<void(AbstractClass&, const std::shared_ptr<Derived>&)>([](AbstractClass& self, const std::shared_ptr<Derived>& derived) { + self.AbstractClass::passShared(derived); + })) + .function("passVal", select_overload<void(AbstractClass&, const val&)>([](AbstractClass& self, const val& v) { + self.AbstractClass::passVal(v); + })) ; function("getAbstractClass", &getAbstractClass); @@ -1216,6 +1245,8 @@ EMSCRIPTEN_BINDINGS(interface_tests) { function("callOptionalMethod", &callOptionalMethod); function("callReturnsSharedPtrMethod", &callReturnsSharedPtrMethod); function("callDifferentArguments", &callDifferentArguments); + function("passShared", &passShared); + function("passVal", &passVal); class_<AbstractClassWithConstructor>("AbstractClassWithConstructor") .allow_subclass<AbstractClassWithConstructorWrapper>("AbstractClassWithConstructorWrapper", constructor<std::string>()) |