Tests passing once we let memory leak. Fix for memory leaks to be carefully enabled.

nishant-rudderstack · nishant-rudderstack · commit 1908f17a5b10 · 2022-06-19T10:47:24.000+05:30
diff --git a/_examples/multireturn/multireturn.go b/_examples/multireturn/multireturn.go
@@ -28,9 +28,12 @@ func SingleWithErrorFunc(throwError bool) error {
 }
 
 /////////////// Double WithoutError Return //////////////
-func DoubleWithoutErrorFunc() (int, int) {
+func DoubleWithoutErrorFunc1() (int, int) {
 	return 200, 300
 }
+func DoubleWithoutErrorFunc2() (string, string) {
+	return "200", "300"
+}
 
 /////////////// Double WithError Return //////////////
 func DoubleWithErrorFunc(throwError bool) (string, error) {
diff --git a/_examples/multireturn/test.py b/_examples/multireturn/test.py
@@ -16,25 +16,28 @@
 print("Single Str WithoutError Return %r" % oneStrResult)
 
 ############### Single WithError Return ##############
-errorFalseResult = multireturn.SingleWithErrorFunc(False)
-print("Single WithError(False) Return %r" % errorFalseResult)
+errorFalseResult1 = multireturn.SingleWithErrorFunc(False)
+print("Single WithError(False) Return %r" % errorFalseResult1)
 
 try:
-	errorTrueResult = multireturn.SingleWithErrorFunc(True)
+	errorTrueResult1 = multireturn.SingleWithErrorFunc(True)
 	print("Failed to throw an exception")
 except RuntimeError as ex:
-	print("Single WithError(True). Exception:%r" % ex)
+	print("Single WithError(True). Exception: %r" % ex)
 
 ############### Double WithoutError Return ##############
-twoResults = multireturn.DoubleWithoutErrorFunc()
-print("Double WithoutError Return (%r, %r)" % twoResults)
+twoResults = multireturn.DoubleWithoutErrorFunc1()
+print("Double WithoutError(Without String) Return (%r, %r)" % twoResults)
+
+twoResults = multireturn.DoubleWithoutErrorFunc2()
+print("Double WithoutError(With String) Return (%r, %r)" % twoResults)
 
 ############### Double WithError Return ##############
 try:
 	value400 = multireturn.DoubleWithErrorFunc(True)
 	print("Failed to throw an exception. Return (%r, %r)." % value400)
 except RuntimeError as ex:
-	print("Double WithError(True). exception Return %r" % ex)
+	print("Double WithError(True). Exception: %r" % ex)
 
 value500 = multireturn.DoubleWithErrorFunc(False)
 print("Double WithError(False) Return %r" % value500)
@@ -48,20 +51,24 @@
 
 ############### Triple With Error Return ##############
 try:
-	(value900, value1000, errorTrueResult) = multireturn.TripleWithErrorFunc(True)
+	(value900, value1000) = multireturn.TripleWithErrorFunc(True)
+	print("Triple WithError(True) Return (%r, %r, %r)" % (value900, value1000))
 except RuntimeError as ex:
-	print("Triple WithError(True). exception Return %r" % ex)
+	print("Triple WithError(True) Exception: %r" % ex)
 
 (value1100, value1200) = multireturn.TripleWithErrorFunc(False)
 print("Triple WithError(False) Return (%r, %r)" % (value1100, value1200))
 
 ############### Triple Struct Return With Error ##############
-(ptr1300, struct1400, errorTrueResult) = multireturn.TripleWithStructWithErrorFunc(True)
-print("Triple WithError(True) Return (%r, %r, %r)" % (ptr1300.P, struct1400.P, errorFalseResult))
+try:
+	(ptr1300, struct1400) = multireturn.TripleWithStructWithErrorFunc(True)
+	print("Triple WithError(True) Return (%r, %r)" % (ptr1300.P, struct1400.P))
+except RuntimeError as ex:
+	print("Triple WithError(True) Exception: %r" % ex)
 
-(value1500, value1600, errorFalseResult) = multireturn.TripleWithStructWithErrorFunc(False)
-print("Triple WithError(False) Return (%r, %r, %r)" % (value1500.P, value1600.P, errorFalseResult))
+(value1500, value1600) = multireturn.TripleWithStructWithErrorFunc(False)
+print("Triple WithError(False) Return (%r, %r)" % (value1500.P, value1600.P))
 
 ############### Triple Interface Return Without Error ##############
 (interface1700, struct1800, ptr1900) = multireturn.TripleWithInterfaceWithoutErrorFunc()
-print("Triple WithError(True) Return (%r, %r, %r)" % (interface1700.P, struct1800.P, ptr1900))
+print("Triple WithoutError() Return (%r, %r, %r)" % (interface1700.Number(), struct1800.P, ptr1900.P))
diff --git a/bind/gen.go b/bind/gen.go
@@ -86,13 +86,17 @@ static inline void gopy_err_handle() {
 		PyErr_Print();
 	}
 }
-static PyObject* Py_BuildValue1(const char *format, void* arg0)
+static PyObject* Py_BuildValue1(char *format, void* arg0)
 {
-	return Py_BuildValue(format, arg0);
+	PyObject *retval = Py_BuildValue(format, arg0);
+	free(format);
+	return retval;
 }
-static PyObject* Py_BuildValue2(const char *format, long long arg0)
+static PyObject* Py_BuildValue2(char *format, long long arg0)
 {
-	return Py_BuildValue(format, arg0);
+	PyObject *retval = Py_BuildValue(format, arg0);
+	free(format);
+	return retval;
 }
 
 %[9]s
@@ -241,19 +245,22 @@ class CheckedFunction(Function):
         failure_cleanup = self._failure_cleanup or None
         self.before_call.write_error_check(check, failure_cleanup)
 
-def add_checked_function(mod, name, retval, params, failure_expression='', *a, **kw):
-    fn = CheckedFunction(name, retval, params, *a, **kw)
-    fn.set_failure_expression(failure_expression)
-    mod._add_function_obj(fn)
-    return fn
-
-def add_checked_string_function(mod, name, retval, params, failure_expression='', *a, **kw):
-    fn = CheckedFunction(name, retval, params, *a, **kw)
-    fn.set_failure_cleanup('if (retval != NULL) free(retval);')
-    fn.after_call.add_cleanup_code('free(retval);')
-    fn.set_failure_expression(failure_expression)
-    mod._add_function_obj(fn)
-    return fn
+def add_checked_function_generator(pyTupleBuilt, retvals_to_free):
+    if not pyTupleBuilt:
+        if retvals_to_free:
+            assert(len(retvals_to_free) == 1)
+            assert(retvals_to_free[0] == 0)
+    def rv_format(format_str, rv):
+        return format_str.format("PyTuple_GetItem(retval, {0})".format(rv))
+    def add_checked_function(mod, name, retval, params, failure_expression='', *a, **kw):
+        fn = CheckedFunction(name, retval, params, *a, **kw)
+        #TODO: Figure out how to free allocated variables. Stop leaking memory.
+        #fn.set_failure_cleanup('\n'.join([rv_format('if ({0} != NULL) free({0});', rv) for rv in retvals_to_free]))
+        #fn.after_call.add_cleanup_code('\n'.join([rv_format('free({0});', rv) for rv in retvals_to_free]))
+        fn.set_failure_expression(failure_expression)
+        mod._add_function_obj(fn)
+        return fn
+    return add_checked_function
 
 mod = Module('_%[1]s')
 mod.add_include('"%[1]s_go.h"')
@@ -384,6 +391,10 @@ build:
 	# generated %[1]s.py python wrapper imports this c-code package
 	%[9]s
 	$(GCC) %[1]s.c %[6]s %[1]s_go$(LIBEXT) -o _%[1]s$(LIBEXT) $(CFLAGS) $(LDFLAGS) -fPIC --shared -w
+
+halfbuild:
+	$(GOBUILD) -buildmode=c-shared -o %[1]s_go$(LIBEXT) %[1]s.go
+	$(GCC) %[1]s.c %[6]s %[1]s_go$(LIBEXT) -o _%[1]s$(LIBEXT) $(CFLAGS) $(LDFLAGS) -fPIC --shared -w
 	
 `
 
diff --git a/bind/gen_func.go b/bind/gen_func.go
@@ -132,19 +132,24 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool {
 	// a function that adds function calls with exception checking.
 	// But given specific return types, we may want to add more
 	// behavior to the wrapped function code gen.
-	addFuncName := "add_checked_function"
+	retvalsToFree := make([]string, 0, npyres)
 	if npyres > 0 {
 		for i := 0; i < npyres; i++ {
 			switch t := res[i].GoType().(type) {
 			case *types.Basic:
 				// string return types need special memory leak patches
 				// to free the allocated char*
 				if t.Kind() == types.String {
-					addFuncName = "add_checked_string_function"
+					retvalsToFree = append(retvalsToFree, strconv.Itoa(i))
 				}
 			}
 		}
 	}
+	pyTupleBuilt := "True"
+	if !buildPyTuple(fsym) {
+		pyTupleBuilt = "False"
+	}
+	addFuncName := "add_checked_function_generator(" + pyTupleBuilt + ", [" + strings.Join(retvalsToFree, ", ") + "])"
 
 	switch {
 	case isMethod:
@@ -506,6 +511,7 @@ if __err != nil {
 				if !isPointer(formatStr) {
 					buildValueFunc = "C.Py_BuildValue2"
 					typeCast = "C.longlong"
+					formatStr = "L"
 				}
 				valueCall := fmt.Sprintf("%s(C.CString(\"%s\"), %s(%s))",
 					buildValueFunc,
diff --git a/main_test.go b/main_test.go
@@ -840,19 +840,20 @@ func TestBindMultiReturn(t *testing.T) {
 		extras: nil,
 		want: []byte(`No Return None
 Single WithoutError Return 100
-Single Str WithoutError Return 150
-Single WithError(False) Return nil
-Single WithError(True). Exception: 'Error'
-Double WithoutError Return (200,300)
-Double WithError(True). Return (400, Error)
-Double WithError(False) Return (500, nil)
+Single Str WithoutError Return '150'
+Single WithError(False) Return None
+Single WithError(True). Exception: RuntimeError('Error')
+Double WithoutError(Without String) Return (200, 300)
+Double WithoutError(With String) Return ('200', '300')
+Double WithError(True). Exception: RuntimeError('Error')
+Double WithError(False) Return '500'
 Triple WithoutError(Without String) Return (600, 700, 800)
-Triple WithoutError(With String) Return (600, 700, 800)
-Triple WithError(True) Return (900, 1000, Error)
-Triple WithError(False) Return (1100, 1200, nil)
-Triple WithError(True) Return (1300, 1400, Error)
-Triple WithError(False) Return (1500, 1600, nil)
-Triple WithError(True) Return (1700, 1800, 1900)
+Triple WithoutError(With String) Return (600, '700', 800)
+Triple WithError(True) Exception: RuntimeError('Error')
+Triple WithError(False) Return (1100, 1200)
+Triple WithError(True) Exception: RuntimeError('Error')
+Triple WithError(False) Return (1500, 1600)
+Triple WithoutError() Return (1700, 1800, 1900)
 `),
 	})
 }

Original file line number	Diff line number	Diff line change
`@@ -28,9 +28,12 @@ func SingleWithErrorFunc(throwError bool) error {`
`28`	`28`	`}`
`29`	`29`
`30`	`30`	`/////////////// Double WithoutError Return //////////////`
`31`		`-func DoubleWithoutErrorFunc() (int, int) {`
	`31`	`+func DoubleWithoutErrorFunc1() (int, int) {`
`32`	`32`	`return 200, 300`
`33`	`33`	`}`
	`34`	`+func DoubleWithoutErrorFunc2() (string, string) {`
	`35`	`+ return "200", "300"`
	`36`	`+}`
`34`	`37`
`35`	`38`	`/////////////// Double WithError Return //////////////`
`36`	`39`	`func DoubleWithErrorFunc(throwError bool) (string, error) {`