fix: proper gil handling, increase testing for symbols & complex numbers

_examples/simple/test_complex_stress.py

@@ -0,0 +1,60 @@
+# Copyright 2026 The go-python Authors.  All rights reserved.
+# Use of this source code is governed by a BSD-style
+# license that can be found in the LICENSE file.
+
+# Regression/stress test for the complex64/complex128 GIL-handling fix.
+#
+# Comp64Add/Comp128Add marshal a raw *C.PyObject on both sides of the call:
+# PyComplex_AsCComplex on the way in, PyComplex_FromDoubles on the way out.
+# Both must run while the GIL is held -- gopy releases the GIL only around
+# the pure-Go call in between. This hammers the round trip from the main
+# thread while a background thread continuously allocates and collects
+# Python objects, so that if the marshalling window ever slipped outside the
+# GIL-held region, refcount/GC corruption would have a chance to surface.
+
+import gc
+import sys
+import threading
+
+import simple as pkg
+
+ITERATIONS = 20000
+STOP = threading.Event()
+
+
+def churn():
+    while not STOP.is_set():
+        _ = [object() for _ in range(100)]
+        gc.collect(0)
+
+
+t = threading.Thread(target=churn, daemon=True)
+t.start()
+
+try:
+    for i in range(ITERATIONS):
+        a = complex(i, -i)
+        b = complex(-i, i * 2)
+        want = a + b
+
+        got64 = pkg.Comp64Add(a, b)
+        # complex64 loses precision relative to Python's complex128
+        # arithmetic; compare with a tolerance.
+        if abs(got64 - want) > 1e-3:
+            print("FAIL: Comp64Add(%s, %s) = %s, want ~%s" % (a, b, got64, want),
+                  file=sys.stderr)
+            sys.exit(1)
+
+        got128 = pkg.Comp128Add(a, b)
+        if got128 != want:
+            print("FAIL: Comp128Add(%s, %s) = %s, want %s" % (a, b, got128, want),
+                  file=sys.stderr)
+            sys.exit(1)
+
+        if i % 500 == 0:
+            gc.collect()
+finally:
+    STOP.set()
+    t.join()
+
+print("OK")

bind/gen_func.go

@@ -216,6 +216,18 @@ func isIfaceHandle(gdoc string) (bool, string) {
 	return false, gdoc
 }
 
+// needsGILForArgMarshal reports whether converting a Python-side argument of
+// this symbol's type touches a raw *C.PyObject on the Go side (e.g.
+// complex64/complex128 via PyComplex_AsCComplex). Such conversions must run
+// while the GIL is still held, not inline in the wrapped call after
+// C.PyEval_SaveThread has released it -- touching a PyObject without the GIL
+// is undefined behavior. The py2go signature closure (isSignature) is
+// excluded: it is invoked later, from inside the wrapped Go call, and already
+// reacquires the GIL itself via PyGILState_Ensure/Release around its body.
+func needsGILForArgMarshal(sym *symbol) bool {
+	return sym.cgoname == "*C.PyObject" && !sym.isSignature() && sym.py2go != ""
+}
+
 func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) {
 	isMethod := (sym != nil)
 	isIface := false
@@ -261,6 +273,24 @@ func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) {
 		}
 	}
 
+	// Convert any argument whose marshalling touches a raw *C.PyObject while
+	// the GIL is still held -- before it is released below for the duration
+	// of the wrapped Go call. The variadic tail is skipped: its "arg" stands
+	// in for the whole trailing slice, not a single marshalled scalar.
+	premarshalled := make(map[int]string)
+	for i, arg := range args {
+		if !needsGILForArgMarshal(arg.sym) {
+			continue
+		}
+		if fsym.isVariadic && i == len(args)-1 {
+			continue
+		}
+		anm := pySafeArg(arg.Name(), i)
+		varnm := fmt.Sprintf("_premarshal%d", i)
+		g.gofile.Printf("%s := %s(%s)%s\n", varnm, arg.sym.py2go, anm, arg.sym.py2goParenEx)
+		premarshalled[i] = varnm
+	}
+
 	g.gofile.Printf("_saved_thread := C.PyEval_SaveThread()\n")
 	if !rvIsErr && nres != 2 {
 		g.gofile.Printf("defer C.PyEval_RestoreThread(_saved_thread)\n")
@@ -304,6 +334,8 @@ if __err != nil {
 			na = fmt.Sprintf(`gopyh.VarFromHandle((gopyh.CGoHandle)(%s), "interface{}")`, anm)
 		case arg.sym.isSignature():
 			na = fmt.Sprintf("%s", arg.sym.py2go)
+		case premarshalled[i] != "":
+			na = premarshalled[i]
 		case arg.sym.py2go != "":
 			na = fmt.Sprintf("%s(%s)%s", arg.sym.py2go, anm, arg.sym.py2goParenEx)
 		default:

bind/symbols.go

@@ -636,8 +636,19 @@ func (sym *symtab) buildTuple(tuple *types.Tuple, varnm string, methvar string)
 	// 	bstr += fmt.Sprintf("}\n")
 	// }
 
+	// PyTuple_New/PyTuple_SetItem below touch raw *C.PyObject values and
+	// must run with the GIL held. This code is spliced verbatim into a
+	// caller-generated function body, so it cannot assume the GIL is
+	// already held there -- bracket it here instead of relying on every
+	// caller to do so correctly (PyGILState_Ensure/Release nest safely, so
+	// this is sound even when the caller already holds the GIL itself).
+	// varnm is declared outside the block so it stays visible to
+	// caller-emitted code that follows (e.g. PyObject_CallObject).
+	//
 	// TODO: more efficient to use strings.Builder here..
-	bstr := fmt.Sprintf("%s := C.PyTuple_New(%d)\n", varnm, sz)
+	bstr := fmt.Sprintf("var %s *C.PyObject\n{\n", varnm)
+	bstr += "_tuple_gstate := C.PyGILState_Ensure()\n"
+	bstr += fmt.Sprintf("%s = C.PyTuple_New(%d)\n", varnm, sz)
 	for i := 0; i < sz; i++ {
 		v := tuple.At(i)
 		typ := v.Type()
@@ -679,6 +690,8 @@ func (sym *symtab) buildTuple(tuple *types.Tuple, varnm string, methvar string)
 			return "", fmt.Errorf("buildTuple: type not handled: %s", typ.String())
 		}
 	}
+	bstr += "C.PyGILState_Release(_tuple_gstate)\n"
+	bstr += "}\n"
 	return bstr, nil
 }
 

bind/symbols_test.go

@@ -0,0 +1,60 @@
+// Copyright 2026 The go-python Authors.  All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package bind
+
+import (
+	"go/token"
+	"go/types"
+	"strings"
+	"testing"
+)
+
+// TestBuildTupleHoldsGIL guards against a regression class already fixed
+// once for complex64/complex128 (see needsGILForArgMarshal in gen_func.go):
+// generated code that touches a raw *C.PyObject must never run outside a
+// PyGILState_Ensure/Release (or equivalent) bracket.
+//
+// buildTuple emits PyTuple_New/PyTuple_SetItem -- pure PyObject manipulation
+// -- as a bare code fragment with no way to know whether its caller already
+// holds the GIL. Its one current caller (addSignatureType's py2go closure)
+// happens to bracket it correctly today, but that's a convention external
+// to buildTuple, not something buildTuple enforces. This test requires
+// buildTuple's own output to be self-bracketing, so a future caller can't
+// reintroduce the complex64/128 bug by forgetting to hold the GIL.
+func TestBuildTupleHoldsGIL(t *testing.T) {
+	tuple := types.NewTuple(
+		types.NewVar(token.NoPos, nil, "i", types.Typ[types.Int]),
+		types.NewVar(token.NoPos, nil, "s", types.Typ[types.String]),
+	)
+
+	got, err := current.buildTuple(tuple, "_fcargs", "_fun_arg")
+	if err != nil {
+		t.Fatalf("buildTuple returned error: %v", err)
+	}
+
+	ensureCount := strings.Count(got, "C.PyGILState_Ensure()")
+	releaseCount := strings.Count(got, "C.PyGILState_Release(")
+	if ensureCount == 0 || releaseCount == 0 {
+		t.Fatalf("buildTuple output does not self-bracket its PyTuple_* calls "+
+			"with PyGILState_Ensure/Release; got:\n%s", got)
+	}
+	if ensureCount != releaseCount {
+		t.Fatalf("mismatched PyGILState_Ensure/Release counts (%d vs %d) in "+
+			"buildTuple output:\n%s", ensureCount, releaseCount, got)
+	}
+
+	ensureIdx := strings.Index(got, "C.PyGILState_Ensure()")
+	firstTupleIdx := strings.Index(got, "C.PyTuple_")
+	lastTupleIdx := strings.LastIndex(got, "C.PyTuple_")
+	releaseIdx := strings.LastIndex(got, "C.PyGILState_Release(")
+
+	if ensureIdx == -1 || firstTupleIdx == -1 || releaseIdx == -1 {
+		t.Fatalf("could not locate expected markers in buildTuple output:\n%s", got)
+	}
+	if !(ensureIdx < firstTupleIdx && lastTupleIdx < releaseIdx) {
+		t.Fatalf("PyTuple_* calls are not nested inside the "+
+			"PyGILState_Ensure/Release bracket in buildTuple output:\n%s", got)
+	}
+}

main_test.go

@@ -851,6 +851,64 @@ func TestGilString(t *testing.T) {
 	}
 }
 
+// TestComplexGILStress is a regression/stress test for the complex64/128
+// GIL-handling fix. Comp64Add/Comp128Add marshal a raw *C.PyObject on both
+// sides of the call (PyComplex_AsCComplex on the argument-read side,
+// PyComplex_FromDoubles on the return side); both must run while the GIL is
+// held, since gopy releases it only around the pure-Go call in between. This
+// repeatedly exercises the round trip under GC/allocation pressure from a
+// background thread so a mis-timed marshalling window has a chance to
+// surface as corruption rather than passing silently.
+func TestComplexGILStress(t *testing.T) {
+	backends := []string{"py3"}
+	for _, be := range backends {
+		vm, ok := testBackends[be]
+		if !ok || vm == "" {
+			t.Logf("Skipped testing backend %s for TestComplexGILStress\n", be)
+			continue
+		}
+		t.Run(be, func(t *testing.T) {
+			cwd, _ := os.Getwd()
+
+			workdir, err := os.MkdirTemp("", "gopy-")
+			if err != nil {
+				t.Fatalf("could not create workdir: %v", err)
+			}
+			defer os.RemoveAll(workdir)
+			defer bind.ResetPackages()
+
+			writeGoMod(t, cwd, workdir)
+			if err := run([]string{"build", "-vm=" + vm, "-output=" + workdir, "-package-prefix", "", "./_examples/simple"}); err != nil {
+				t.Fatalf("error building simple: %v", err)
+			}
+
+			tstDst := filepath.Join(workdir, "test_complex_stress.py")
+			if err := copyCmd(filepath.Join(cwd, "_examples/simple/test_complex_stress.py"), tstDst); err != nil {
+				t.Fatalf("error copying test_complex_stress.py: %v", err)
+			}
+
+			env := make([]string, len(testEnvironment))
+			copy(env, testEnvironment)
+			env = append(env, fmt.Sprintf("PYTHONPATH=%s", workdir))
+
+			cmd := exec.Command(vm, "./test_complex_stress.py")
+			cmd.Env = env
+			cmd.Dir = workdir
+			cmd.Stdin = os.Stdin
+			buf, err := cmd.CombinedOutput()
+			if err != nil {
+				t.Fatalf("error running python module: err=%v\n%s", err, string(buf))
+			}
+
+			got := strings.Replace(string(buf), "\r\n", "\n", -1)
+			want := "OK\n"
+			if got != want {
+				t.Fatalf("got:\n%s\nwant:\n%s", got, want)
+			}
+		})
+	}
+}
+
 func TestPackagePrefix(t *testing.T) {
 	// t.Parallel()
 	path := "_examples/package/mypkg"