diff options
| -rw-r--r-- | src/embind/emval.js | 35 | ||||
| -rw-r--r-- | system/include/emscripten/val.h | 65 | ||||
| -rw-r--r-- | system/include/emscripten/wire.h | 31 | ||||
| -rw-r--r-- | tests/embind/embind.test.js | 15 |
4 files changed, 77 insertions, 69 deletions
diff --git a/src/embind/emval.js b/src/embind/emval.js index d4e4271d..039f1d61 100644 --- a/src/embind/emval.js +++ b/src/embind/emval.js @@ -79,6 +79,12 @@ function __emval_decref(handle) { } } +function __emval_run_destructors(handle) { + var destructors = _emval_handle_array[handle].value; + runDestructors(destructors); + __emval_decref(handle); +} + function __emval_new_array() { return __emval_register([]); } @@ -199,12 +205,12 @@ function __emval_set_property(handle, key, value) { _emval_handle_array[handle].value[_emval_handle_array[key].value] = _emval_handle_array[value].value; } -function __emval_as(handle, returnType, runDestructorsRef) { +function __emval_as(handle, returnType, destructorsRef) { requireHandle(handle); returnType = requireRegisteredType(returnType, 'emval::as'); var destructors = []; - var rd = __emval_register(runDestructors.bind(undefined, destructors)); - HEAP32[runDestructorsRef >> 2] = rd; + var rd = __emval_register(destructors); + HEAP32[destructorsRef >> 2] = rd; return returnType['toWireType'](destructors, _emval_handle_array[handle].value); } @@ -243,14 +249,20 @@ function lookupTypes(argCount, argTypes, argWireTypes) { return a; } +function allocateDestructors(destructorsRef) { + var destructors = []; + HEAP32[destructorsRef >> 2] = __emval_register(destructors); + return destructors; +} + function __emval_get_method_caller(argCount, argTypes) { var types = lookupTypes(argCount, argTypes); var retType = types[0]; var signatureName = retType.name + "_$" + types.slice(1).map(function (t) { return t.name; }).join("_") + "$"; - var args1 = ["addFunction", "createNamedFunction", "requireHandle", "getStringOrSymbol", "_emval_handle_array", "retType"]; - var args2 = [Runtime.addFunction, createNamedFunction, requireHandle, getStringOrSymbol, _emval_handle_array, retType]; + var args1 = ["addFunction", "createNamedFunction", "requireHandle", "getStringOrSymbol", "_emval_handle_array", "retType", "allocateDestructors"]; + var args2 = [Runtime.addFunction, createNamedFunction, requireHandle, getStringOrSymbol, _emval_handle_array, retType, allocateDestructors]; var argsList = ""; // 'arg0, arg1, arg2, ... , argN' var argsListWired = ""; // 'arg0Wired, ..., argNWired' @@ -262,16 +274,17 @@ function __emval_get_method_caller(argCount, argTypes) { } var invokerFnBody = - "return addFunction(createNamedFunction('" + signatureName + "', function (handle, name" + argsListWired + ") {\n" + - "requireHandle(handle);\n" + - "name = getStringOrSymbol(name);\n"; + "return addFunction(createNamedFunction('" + signatureName + "', function (handle, name, destructorsRef" + argsListWired + ") {\n" + + " requireHandle(handle);\n" + + " name = getStringOrSymbol(name);\n"; for (var i = 0; i < argCount - 1; ++i) { - invokerFnBody += "var arg" + i + " = argType" + i + ".fromWireType(arg" + i + "Wired);\n"; + invokerFnBody += " var arg" + i + " = argType" + i + ".fromWireType(arg" + i + "Wired);\n"; } invokerFnBody += - "var obj = _emval_handle_array[handle].value;\n" + - "return retType.toWireType(null, obj[name](" + argsList + "));\n" + + " var obj = _emval_handle_array[handle].value;\n" + + " var rv = obj[name](" + argsList + ");\n" + + " return retType.toWireType(allocateDestructors(destructorsRef), rv);\n" + "}));\n"; args1.push(invokerFnBody); diff --git a/system/include/emscripten/val.h b/system/include/emscripten/val.h index 5a04d30f..19b1beb1 100644 --- a/system/include/emscripten/val.h +++ b/system/include/emscripten/val.h @@ -11,6 +11,7 @@ namespace emscripten { void _emval_register_symbol(const char*); typedef struct _EM_VAL* EM_VAL; + typedef struct _EM_DESTRUCTORS* EM_DESTRUCTORS; // TODO: functions returning this are reinterpret_cast // into the correct return type. this needs some thought @@ -20,6 +21,8 @@ namespace emscripten { void _emval_incref(EM_VAL value); void _emval_decref(EM_VAL value); + void _emval_run_destructors(EM_DESTRUCTORS handle); + EM_VAL _emval_new_array(); EM_VAL _emval_new_object(); EM_VAL _emval_undefined(); @@ -37,7 +40,7 @@ namespace emscripten { EM_VAL _emval_get_module_property(const char* name); EM_VAL _emval_get_property(EM_VAL object, EM_VAL key); void _emval_set_property(EM_VAL object, EM_VAL key, EM_VAL value); - _POLYMORPHIC_RESULT _emval_as(EM_VAL value, TYPEID returnType, EM_VAL* runDestructors); + _POLYMORPHIC_RESULT _emval_as(EM_VAL value, TYPEID returnType, EM_DESTRUCTORS* runDestructors); EM_VAL _emval_call( EM_VAL value, @@ -63,7 +66,11 @@ namespace emscripten { template<typename ReturnType, typename... Args> struct Signature { - typedef typename BindingType<ReturnType>::WireType (*MethodCaller)(EM_VAL value, const char* methodName, typename BindingType<Args>::WireType...); + typedef typename BindingType<ReturnType>::WireType (*MethodCaller)( + EM_VAL value, + const char* methodName, + EM_DESTRUCTORS* destructors, + typename BindingType<Args>::WireType...); static MethodCaller get_method_caller() { static MethodCaller fp = reinterpret_cast<MethodCaller>(init_method_caller()); @@ -77,15 +84,34 @@ namespace emscripten { } }; + struct DestructorsRunner { + public: + explicit DestructorsRunner(EM_DESTRUCTORS d) + : destructors(d) + {} + ~DestructorsRunner() { + _emval_run_destructors(destructors); + } + + DestructorsRunner(const DestructorsRunner&) = delete; + void operator=(const DestructorsRunner&) = delete; + + private: + EM_DESTRUCTORS destructors; + }; + template<typename ReturnType, typename... Args> struct MethodCaller { static ReturnType call(EM_VAL handle, const char* methodName, Args&&... args) { auto caller = Signature<ReturnType, Args...>::get_method_caller(); + + EM_DESTRUCTORS destructors; auto wireType = caller( handle, methodName, + &destructors, toWireType(std::forward<Args>(args))...); - WireDeleter<ReturnType> deleter(wireType); + DestructorsRunner rd(destructors); return BindingType<ReturnType>::fromWireType(wireType); } }; @@ -94,28 +120,17 @@ namespace emscripten { struct MethodCaller<void, Args...> { static void call(EM_VAL handle, const char* methodName, Args&&... args) { auto caller = Signature<void, Args...>::get_method_caller(); - return caller( + + EM_DESTRUCTORS destructors; + caller( handle, methodName, + &destructors, toWireType(std::forward<Args>(args))...); + DestructorsRunner rd(destructors); + // void requires no translation } }; - - struct DestructorsRunner { - public: - DestructorsRunner(EM_VAL v) - : dr(v) - {} - DestructorsRunner(const DestructorsRunner&) = delete; - void operator=(const DestructorsRunner&) = delete; - ~DestructorsRunner() { - EM_VAL rv = _emval_call(dr, 0, 0); - _emval_decref(rv); // TODO: if we had an _emval_call_void we wouldn't need this - _emval_decref(dr); - } - private: - EM_VAL dr; - }; } #define EMSCRIPTEN_SYMBOL(name) \ @@ -285,12 +300,12 @@ namespace emscripten { typedef typename BT::WireType (*TypedAs)( EM_VAL value, TYPEID returnType, - EM_VAL* runDestructors); + EM_DESTRUCTORS* runDestructors); TypedAs typedAs = reinterpret_cast<TypedAs>(&_emval_as); - EM_VAL runDestructors; - typename BT::WireType wt = typedAs(handle, TypeID<T>::get(), &runDestructors); - DestructorsRunner dr(runDestructors); + EM_DESTRUCTORS destructors; + typename BT::WireType wt = typedAs(handle, TypeID<T>::get(), &destructors); + DestructorsRunner dr(destructors); return BT::fromWireType(wt); } @@ -316,8 +331,6 @@ namespace emscripten { static val fromWireType(WireType v) { return val::take_ownership(v); } - static void destroy(WireType v) { - } }; } diff --git a/system/include/emscripten/wire.h b/system/include/emscripten/wire.h index 70deb2c7..c3ce8dd0 100644 --- a/system/include/emscripten/wire.h +++ b/system/include/emscripten/wire.h @@ -131,8 +131,6 @@ namespace emscripten { constexpr static type fromWireType(WireType v) { \ return v; \ } \ - static void destroy(WireType) { \ - } \ } EMSCRIPTEN_DEFINE_NATIVE_BINDING_TYPE(char); @@ -161,8 +159,6 @@ namespace emscripten { static bool fromWireType(WireType wt) { return wt; } - static void destroy(WireType) { - } }; template<> @@ -180,9 +176,6 @@ namespace emscripten { static std::string fromWireType(WireType v) { return std::string(v->data, v->length); } - static void destroy(WireType v) { - free(v); - } }; template<> @@ -200,9 +193,6 @@ namespace emscripten { static std::wstring fromWireType(WireType v) { return std::wstring(v->data, v->length); } - static void destroy(WireType v) { - free(v); - } }; template<typename T> @@ -255,10 +245,6 @@ namespace emscripten { static ActualT& fromWireType(WireType p) { return *p; } - - static void destroy(WireType p) { - delete p; - } }; // Is this necessary? @@ -281,8 +267,6 @@ namespace emscripten { static Enum fromWireType(WireType v) { return v; } - static void destroy(WireType) { - } }; // catch-all generic binding @@ -297,21 +281,6 @@ namespace emscripten { auto toWireType(T&& v) -> typename BindingType<T>::WireType { return BindingType<T>::toWireType(std::forward<T>(v)); } - - template<typename T> - struct WireDeleter { - typedef typename BindingType<T>::WireType WireType; - - WireDeleter(WireType wt) - : wt(wt) - {} - - ~WireDeleter() { - BindingType<T>::destroy(wt); - } - - WireType wt; - }; } struct memory_view { diff --git a/tests/embind/embind.test.js b/tests/embind/embind.test.js index a36c8b08..5ca972be 100644 --- a/tests/embind/embind.test.js +++ b/tests/embind/embind.test.js @@ -1591,7 +1591,7 @@ module({ test("returning a new shared pointer from interfaces implemented in JS code does not leak", function() { var impl = cm.AbstractClass.implement({ returnsSharedPtr: function() { - return cm.embind_test_return_smart_derived_ptr(); + return cm.embind_test_return_smart_derived_ptr().deleteLater(); } }); cm.callReturnsSharedPtrMethod(impl); @@ -1925,6 +1925,19 @@ module({ a.delete(); }); }); + + test("returning a cached new shared pointer from interfaces implemented in JS code does not leak", function() { + var derived = cm.embind_test_return_smart_derived_ptr(); + var impl = cm.AbstractClass.implement({ + returnsSharedPtr: function() { + return derived; + } + }); + cm.callReturnsSharedPtrMethod(impl); + impl.delete(); + derived.delete(); + // Let the memory leak test superfixture check that no leaks occurred. + }); }); /* global run_all_tests */ |