un-skip TestBindCgoPackage & expand CI matrix

* fix GIL ordering in generated CGo wrappers: restore GIL before go2py converters, use add_checked_string_function for C.CString return paths, and fix off-by-one in cgo.go malloc call
* remove skip for TestBindSimple & TestBindCgoPackage
* update Go version matrix in CI matrix

Author: b-long

.github/workflows/ci.yml

@@ -22,7 +22,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        go-version: [1.25.x, 1.24.x, 1.22.x, 1.21.x]
+        go-version: [1.25.x, 1.24.x, 1.23.x, 1.22.x, 1.21.x]
         platform: [ubuntu-latest, windows-latest]
         #platform: [ubuntu-latest, macos-latest, windows-latest]
     runs-on: ${{ matrix.platform }}

_examples/cgo/cgo.go

@@ -9,7 +9,7 @@ package cgo
 //#include <string.h>
 //#include <stdlib.h>
 //const char* cpkg_sprintf(const char *str) {
-//  char *o = (char*)malloc(strlen(str));
+//  char *o = (char*)malloc(strlen(str) + 1);
 //	sprintf(o, "%s", str);
 //	return o;
 //}

bind/gen_func.go

@@ -263,7 +263,18 @@ func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) {
 
 	// release GIL
 	g.gofile.Printf("_saved_thread := C.PyEval_SaveThread()\n")
-	if !rvIsErr && nres != 2 {
+	// Use defer only when there is no go2py return conversion that calls
+	// Python C API (which requires the GIL to already be reacquired).
+	// For go2py returns (excluding handle types) and error/multi-return cases,
+	// we restore explicitly. Handle types use handleFromPtr which is pure Go
+	// and doesn't need the GIL, so they can keep using defer.
+	nresForDefer := nres
+	// hasRetCvt fires when go2py != "" and NOT (hasHandle && !isPtrOrIface).
+	// Suppress defer in exactly those cases so the explicit restore below is the only one.
+	if nres > 0 && res[0].sym.go2py != "" && !(res[0].sym.hasHandle() && !res[0].sym.isPtrOrIface()) {
+		nresForDefer = 2 // treat like nres==2: no defer, explicit restore
+	}
+	if !rvIsErr && nresForDefer != 2 {
 		// reacquire GIL after return
 		g.gofile.Printf("defer C.PyEval_RestoreThread(_saved_thread)\n")
 	}
@@ -357,8 +368,8 @@ if __err != nil {
 		g.pywrap.Printf("_%s.%s(", pkgname, mnm)
 	}
 
-	hasRetCvt := false
 	hasAddrOfTmp := false
+	hasRetCvt := false
 	if nres > 0 {
 		ret := res[0]
 		switch {
@@ -370,8 +381,10 @@ if __err != nil {
 			hasAddrOfTmp = true
 			g.gofile.Printf("cret := ")
 		case ret.sym.go2py != "":
+			// Assign to cret first; we'll restore the GIL before calling go2py
+			// so that Python C API functions inside go2py run with the GIL held.
 			hasRetCvt = true
-			g.gofile.Printf("return %s(", ret.sym.go2py)
+			g.gofile.Printf("cret := ")
 		default:
 			g.gofile.Printf("return ")
 		}
@@ -394,11 +407,6 @@ if __err != nil {
 	} else {
 		funCall = fmt.Sprintf("%s(%s)", fsym.GoFmt(), strings.Join(callArgs, ", "))
 	}
-	if hasRetCvt {
-		ret := res[0]
-		funCall += fmt.Sprintf(")%s", ret.sym.go2pyParenEx)
-	}
-
 	if nres == 0 {
 		g.gofile.Printf("if boolPyToGo(goRun) {\n")
 		g.gofile.Indent()
@@ -413,6 +421,13 @@ if __err != nil {
 		g.gofile.Printf("%s\n", funCall)
 	}
 
+	if hasRetCvt {
+		// Restore GIL before calling go2py, which may call Python C API.
+		ret := res[0]
+		g.gofile.Printf("C.PyEval_RestoreThread(_saved_thread)\n")
+		g.gofile.Printf("return %s(cret)%s\n", ret.sym.go2py, ret.sym.go2pyParenEx)
+	}
+
 	if rvIsErr || nres == 2 {
 		g.gofile.Printf("\n")
 		// reacquire GIL

bind/gen_map.go

@@ -317,7 +317,11 @@ otherwise parameter is a python list that we copy from
 		g.gofile.Outdent()
 		g.gofile.Printf("}\n\n")
 
-		g.pybuild.Printf("mod.add_function('%s_elem', retval('%s'), [param('%s', 'handle'), param('%s', '_ky')])\n", slNm, esym.cpyname, PyHandle, ksym.cpyname)
+		if esym.go2py == "C.CString" {
+			g.pybuild.Printf("add_checked_string_function(mod, '%s_elem', retval('%s'), [param('%s', 'handle'), param('%s', '_ky')])\n", slNm, esym.cpyname, PyHandle, ksym.cpyname)
+		} else {
+			g.pybuild.Printf("mod.add_function('%s_elem', retval('%s'), [param('%s', 'handle'), param('%s', '_ky')])\n", slNm, esym.cpyname, PyHandle, ksym.cpyname)
+		}
 
 		// contains
 		g.gofile.Printf("//export %s_contains\n", slNm)

bind/gen_slice.go

@@ -345,7 +345,11 @@ otherwise parameter is a python list that we copy from
 			caller_owns_ret = ", caller_owns_return=True"
 			transfer_ownership = ", transfer_ownership=False"
 		}
-		g.pybuild.Printf("mod.add_function('%s_elem', retval('%s'%s), [param('%s', 'handle'), param('int', 'idx')])\n", slNm, esym.cpyname, caller_owns_ret, PyHandle)
+		if esym.go2py == "C.CString" {
+			g.pybuild.Printf("add_checked_string_function(mod, '%s_elem', retval('%s'), [param('%s', 'handle'), param('int', 'idx')])\n", slNm, esym.cpyname, PyHandle)
+		} else {
+			g.pybuild.Printf("mod.add_function('%s_elem', retval('%s'%s), [param('%s', 'handle'), param('int', 'idx')])\n", slNm, esym.cpyname, caller_owns_ret, PyHandle)
+		}
 
 		if slc.isSlice() {
 			g.gofile.Printf("//export %s_subslice\n", slNm)

bind/gen_struct.go

@@ -227,7 +227,11 @@ func (g *pyGen) genStructMemberGetter(s *Struct, i int, f types.Object) {
 	g.gofile.Outdent()
 	g.gofile.Printf("}\n\n")
 
-	g.pybuild.Printf("mod.add_function('%s', retval('%s'), [param('%s', 'handle')])\n", cgoFn, ret.cpyname, PyHandle)
+	if ret.go2py == "C.CString" {
+		g.pybuild.Printf("add_checked_string_function(mod, '%s', retval('%s'), [param('%s', 'handle')])\n", cgoFn, ret.cpyname, PyHandle)
+	} else {
+		g.pybuild.Printf("mod.add_function('%s', retval('%s'), [param('%s', 'handle')])\n", cgoFn, ret.cpyname, PyHandle)
+	}
 }
 
 func (g *pyGen) genStructMemberSetter(s *Struct, i int, f types.Object) {

main_test.go

@@ -316,7 +316,6 @@ OK
 }
 
 func TestBindSimple(t *testing.T) {
-	t.Skip("Skipping due to Go 1.21+ CGO issue (see https://github.com/go-python/gopy/issues/370)")
 	// t.Parallel()
 	path := "_examples/simple"
 	testPkg(t, pkg{
@@ -546,7 +545,6 @@ OK
 }
 
 func TestBindCgoPackage(t *testing.T) {
-	t.Skip("Skipping due to Go 1.21+ CGO issue (see https://github.com/go-python/gopy/issues/370)")
 	// t.Parallel()
 	path := "_examples/cgo"
 	testPkg(t, pkg{