diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e232aa0c..79cca444 100644 --- a/.github/workflows/ci.yml +++ b/.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 }} diff --git a/_examples/cgo/cgo.go b/_examples/cgo/cgo.go index 98a64a31..4119d47a 100644 --- a/_examples/cgo/cgo.go +++ b/_examples/cgo/cgo.go @@ -9,7 +9,7 @@ package cgo //#include //#include //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; //} diff --git a/bind/gen_func.go b/bind/gen_func.go index 8f2e606e..75aacfd7 100644 --- a/bind/gen_func.go +++ b/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 diff --git a/bind/gen_map.go b/bind/gen_map.go index f65111fa..a7fef2b3 100644 --- a/bind/gen_map.go +++ b/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) diff --git a/bind/gen_slice.go b/bind/gen_slice.go index 8df9dedb..9233245f 100644 --- a/bind/gen_slice.go +++ b/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) diff --git a/bind/gen_struct.go b/bind/gen_struct.go index b50ddfc5..9aab31e4 100644 --- a/bind/gen_struct.go +++ b/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) { diff --git a/main_test.go b/main_test.go index c5f6c29e..e7d4292c 100644 --- a/main_test.go +++ b/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{