Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 88 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ jobs:
strategy:
fail-fast: false
matrix:
go-version: [1.25.x, 1.24.x, 1.22.x, 1.21.x]
platform: [ubuntu-latest, windows-latest]
go-version: [1.25.x, 1.24.x, 1.23.x, 1.22.x, 1.21.x]
platform: [ubuntu-latest, windows-latest, macos-15-intel]
#platform: [ubuntu-latest, macos-latest, windows-latest]
runs-on: ${{ matrix.platform }}
steps:
Expand Down Expand Up @@ -59,6 +59,13 @@ jobs:
# install goimports
go install golang.org/x/tools/cmd/goimports@latest

- name: Install macOS packages
if: matrix.platform == 'macos-15-intel'
run: |
# install pybindgen
python3 -m pip install --user -U pybindgen
# install goimports
go install golang.org/x/tools/cmd/goimports@latest

- name: Build-Linux
if: matrix.platform == 'ubuntu-latest'
Expand All @@ -77,6 +84,85 @@ jobs:
if: matrix.platform == 'windows-latest'
run: |
go test -v ./...

- name: Build-macOS
if: matrix.platform == 'macos-15-intel'
run: |
make
- name: Test macOS
if: matrix.platform == 'macos-15-intel'
run: |
make test

- name: Symbol diagnostic (macOS Intel)
if: always() && matrix.platform == 'macos-15-intel'
continue-on-error: true
env:
GOFLAGS: "-mod=mod"
run: |
echo "=== building test packages for symbol analysis ==="
go build -o /tmp/gopy_diag .
DIAGDIR=$(mktemp -d)
CWD=$(pwd)
mkdir -p "$DIAGDIR/gilstring" "$DIAGDIR/simple"
printf 'module dummy\nrequire github.com/go-python/gopy v0.0.0\nreplace github.com/go-python/gopy => %s\n' \
"$CWD" > "$DIAGDIR/gilstring/go.mod"
printf 'module dummy\nrequire github.com/go-python/gopy v0.0.0\nreplace github.com/go-python/gopy => %s\n' \
"$CWD" > "$DIAGDIR/simple/go.mod"
/tmp/gopy_diag build -vm=python3 -output="$DIAGDIR/gilstring" ./_examples/gilstring 2>&1 | tail -3
/tmp/gopy_diag build -vm=python3 -output="$DIAGDIR/simple" ./_examples/simple 2>&1 | tail -3

GIL_SO=$(find "$DIAGDIR/gilstring" -name "_gilstring*.so" 2>/dev/null | head -1)
SIM_SO=$(find "$DIAGDIR/simple" -name "_simple*.so" 2>/dev/null | head -1)
echo "gilstring: $GIL_SO"
echo "simple: $SIM_SO"
[ -n "$GIL_SO" ] && [ -n "$SIM_SO" ] || { echo "SKIP: .so files not found"; exit 0; }

echo ""
echo "=== shared dynamic symbols ==="
nm "$GIL_SO" | awk '$2~/^[A-Z]$/{print $3}' | sort > /tmp/diag_g1.txt
nm "$SIM_SO" | awk '$2~/^[A-Z]$/{print $3}' | sort > /tmp/diag_g2.txt
echo "total shared: $(comm -12 /tmp/diag_g1.txt /tmp/diag_g2.txt | wc -l | tr -d ' ')"
echo "critical CGo bridge symbols shared between both .so files:"
comm -12 /tmp/diag_g1.txt /tmp/diag_g2.txt \
| grep -E "crosscall|cgo_topofstack|x_cgo_init|x_cgo_inittls|cgo_yield|cgo_panic" \
|| echo " (none)"

echo ""
echo "=== indirect symbol table / PLT stubs in gilstring (otool -Iv) ==="
TOTAL=$(otool -Iv "$GIL_SO" | grep -c '0x' || true)
echo "total indirect entries: $TOTAL"
otool -Iv "$GIL_SO" \
| grep -E "crosscall|cgo_topofstack|x_cgo_init|x_cgo_inittls|cgo_yield|cgo_panic" \
|| echo " (none of the critical CGo symbols appear as PLT stubs)"

echo ""
echo "=== indirect symbol table / PLT stubs in simple (otool -Iv) ==="
TOTAL=$(otool -Iv "$SIM_SO" | grep -c '0x' || true)
echo "total indirect entries: $TOTAL"
otool -Iv "$SIM_SO" \
| grep -E "crosscall|cgo_topofstack|x_cgo_init|x_cgo_inittls|cgo_yield|cgo_panic" \
|| echo " (none of the critical CGo symbols appear as PLT stubs)"

echo ""
echo "=== runtime: which library wins in the global namespace ==="
GIL_SO="$GIL_SO" SIM_SO="$SIM_SO" python3 << 'PYEOF'
import ctypes, os
GLOBAL = getattr(ctypes, 'RTLD_GLOBAL', 8)
gil = ctypes.CDLL(os.environ['GIL_SO'], mode=GLOBAL)
sim = ctypes.CDLL(os.environ['SIM_SO'], mode=GLOBAL)
glb = ctypes.CDLL(None)
for s in ['crosscall2', '_cgo_topofstack', 'x_cgo_init', 'x_cgo_inittls', '_cgo_yield', 'x_cgo_mmap']:
try:
a1 = ctypes.cast(getattr(gil, s), ctypes.c_void_p).value
a2 = ctypes.cast(getattr(sim, s), ctypes.c_void_p).value
ag = ctypes.cast(getattr(glb, s), ctypes.c_void_p).value
who = 'gilstring' if ag == a1 else ('simple' if ag == a2 else 'other')
print(' %-35s -> %-12s (global=%s)' % (s, who, hex(ag) if ag else 'N/A'))
except Exception as e:
print(' %-35s error: %s' % (s, e))
PYEOF

- name: Upload-Coverage
if: matrix.platform == 'ubuntu-latest'
uses: codecov/codecov-action@v4
1 change: 1 addition & 0 deletions SUPPORT_MATRIX.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ _examples/consts | yes
_examples/cstrings | yes
_examples/empty | yes
_examples/funcs | yes
_examples/gilstring | yes
_examples/gobytes | yes
_examples/gopygc | yes
_examples/gostrings | yes
Expand Down
2 changes: 1 addition & 1 deletion _examples/cgo/cgo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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;
//}
Expand Down
8 changes: 8 additions & 0 deletions _examples/cgo/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,12 @@
print("cgo.Hi()= %s" % repr(cgo.Hi()).lstrip('u'))
print("cgo.Hello(you)= %s" % repr(cgo.Hello("you")).lstrip('u'))

# Regression test for GIL ordering bug (issue #370 / PR #386):
# go functions returning string (go2py=C.CString) previously called C.CString
# without holding the GIL, causing crashes under repeated calls.
for _ in range(5000):
assert cgo.Hi() == 'hi from go\n'
assert cgo.Hello("world") == 'hello world from go\n'
print("stress OK")

print("OK")
14 changes: 14 additions & 0 deletions _examples/gilstring/gilstring.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// 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 gilstring is a regression test for the GIL ordering bug (issue #370).
// It mirrors the exact reproduction from the issue report: a string-returning
// function called alongside an integer function from a second extension in the
// same Python process, which triggers crashes under repeated calls.
package gilstring

import "fmt"

// Hello returns a greeting string, mirroring hi.Hello from the issue report.
func Hello(s string) string { return fmt.Sprintf("Hello, %s!", s) }
18 changes: 18 additions & 0 deletions _examples/gilstring/test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# 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.

## py2/py3 compat
from __future__ import print_function

# Regression test for GIL ordering bug (issue #370).
# Exact reproduction from the issue report: two separately-built gopy
# extensions loaded in the same process, with calls interleaved in a loop.
from gilstring.gilstring import Hello
from simple.simple import Add

for _ in range(5000):
Add(2, 2)
Hello('hi')

print("OK")
51 changes: 51 additions & 0 deletions bind/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,42 @@ static inline void gopy_err_handle() {
PyErr_Print();
}
}
// gopy_lock / gopy_unlock serialize all gopy extension calls to prevent
// concurrent Go runtimes in the same process from corrupting each other's
// GC sweep-generation counters (issue #370 / #385).
// On non-Windows, the mutex lives in builtins._gopy_global_mu so every
// extension loaded in the same Python process shares the same instance.
// gopy_ensure_mu() must be called while the Python GIL is held (before
// each PyEval_SaveThread), after which gopy_lock/unlock need no GIL.
#ifndef _WIN32
#include <pthread.h>
#include <stdlib.h>
static pthread_mutex_t *_gopy_mu = NULL;
static void gopy_ensure_mu(void) {
if (_gopy_mu) return;
PyObject *bi = PyImport_ImportModule("builtins");
if (!bi) { PyErr_Clear(); return; }
PyObject *cap = PyObject_GetAttrString(bi, "_gopy_global_mu");
if (cap && PyCapsule_CheckExact(cap)) {
_gopy_mu = (pthread_mutex_t *)PyCapsule_GetPointer(cap, "gopy.global_mu");
Py_DECREF(cap);
} else {
PyErr_Clear();
pthread_mutex_t *mu = (pthread_mutex_t *)malloc(sizeof(pthread_mutex_t));
pthread_mutex_init(mu, NULL);
PyObject *nc = PyCapsule_New(mu, "gopy.global_mu", NULL);
if (nc) { PyObject_SetAttrString(bi, "_gopy_global_mu", nc); Py_DECREF(nc); }
_gopy_mu = mu;
}
Py_DECREF(bi);
}
static inline void gopy_lock(void) { if (_gopy_mu) pthread_mutex_lock(_gopy_mu); }
static inline void gopy_unlock(void) { if (_gopy_mu) pthread_mutex_unlock(_gopy_mu); }
#else
static void gopy_ensure_mu(void) {}
static inline void gopy_lock(void) {}
static inline void gopy_unlock(void) {}
#endif
%[8]s
*/
import "C"
Expand Down Expand Up @@ -281,7 +317,22 @@ except ImportError:
cwd = os.getcwd()
currentdir = os.path.dirname(os.path.abspath(inspect.getfile(inspect.currentframe())))
os.chdir(currentdir)
# Load the extension with RTLD_LOCAL so each gopy .so keeps its own copy of
# the Go runtime globals (mheap_, TLS keys, etc.). On macOS, Python's default
# dlopen flags include RTLD_GLOBAL which causes symbol interposition across
# independently-built Go runtimes loaded in the same process (issue #385).
if hasattr(sys, 'getdlopenflags'):
try:
import ctypes as _gopy_ctypes
_gopy_saved_flags = sys.getdlopenflags()
sys.setdlopenflags(_gopy_saved_flags & ~getattr(_gopy_ctypes, 'RTLD_GLOBAL', 0))
except Exception:
_gopy_saved_flags = None
else:
_gopy_saved_flags = None
%[6]s
if _gopy_saved_flags is not None:
sys.setdlopenflags(_gopy_saved_flags)
os.chdir(cwd)

# to use this code in your end-user python file, import it as follows:
Expand Down
62 changes: 59 additions & 3 deletions bind/gen_func.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,11 +261,63 @@ func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) {
}
}

// release GIL
// Idea 1 (fix for issue #370): C→Go argument conversions such as C.GoString
// call runtime.gostring → mallocgc, which can corrupt Go's GC sweep-generation
// counter when executed inside a CGo callback without the GIL held — causing
// "fatal error: bad sweepgen in refill" on Go ≥1.24 / x86_64.
//
// The fix pre-converts each C argument into a Go variable *before* calling
// PyEval_SaveThread, so that any Go heap allocation happens while the GIL is
// held and the Go runtime is in a consistent state. The GIL is released via
// PyGILState_Ensure/Release (not the SaveThread mechanism) so the two paths
// are balanced independently.
//
// Idea 2 (possible future optimisation): Replace C.GoString with
// unsafe.String((*byte)(unsafe.Pointer(cs)), C.strlen(cs))
// for each string parameter. unsafe.String constructs a Go string header
// directly from the C pointer without any heap allocation, so the mallocgc
// call that triggers the sweep-gen check never happens. This is faster
// (no malloc) and avoids the bug at its source, but requires that the
// resulting string does not escape the CGo callback frame — storing it
// beyond the call would be a use-after-free. Both fixes together provide
// defence in depth.

// Pre-convert any C arguments that need a py2go conversion (e.g. C.GoString)
// while the GIL is still held. Record the variable name to use in the call.
preConvertedArgs := map[int]string{} // arg index → temp variable name
needsGILForArgs := false
for _, arg := range args {
if arg.sym.py2go != "" && !arg.sym.isSignature() && !(ifchandle && arg.sym.goname == "interface{}") {
needsGILForArgs = true
break
}
}
if needsGILForArgs {
g.gofile.Printf("_gstate := C.PyGILState_Ensure()\n")
for i, arg := range args {
if arg.sym.py2go != "" && !arg.sym.isSignature() && !(ifchandle && arg.sym.goname == "interface{}") {
anm := pySafeArg(arg.Name(), i)
tmpVar := fmt.Sprintf("_arg%d", i)
g.gofile.Printf("%s := %s(%s)%s\n", tmpVar, arg.sym.py2go, anm, arg.sym.py2goParenEx)
preConvertedArgs[i] = tmpVar
}
}
g.gofile.Printf("C.PyGILState_Release(_gstate)\n")
}

// Ensure the process-wide mutex is initialized (lazy, GIL must be held).
g.gofile.Printf("C.gopy_ensure_mu()\n")
// release GIL then acquire the process-wide runtime mutex.
// Ordering: GIL released first so that a second Python thread blocked on
// the mutex does not hold the GIL while we try to reclaim it, which would
// deadlock. gopy_unlock must run before PyEval_RestoreThread (LIFO defer).
g.gofile.Printf("_saved_thread := C.PyEval_SaveThread()\n")
g.gofile.Printf("C.gopy_lock()\n")
if !rvIsErr && nres != 2 {
// reacquire GIL after return
// PyEval_RestoreThread deferred first → runs last (LIFO).
g.gofile.Printf("defer C.PyEval_RestoreThread(_saved_thread)\n")
// gopy_unlock deferred second → runs first (LIFO): release mutex before reclaiming GIL.
g.gofile.Printf("defer C.gopy_unlock()\n")
}

if isMethod {
Expand Down Expand Up @@ -306,6 +358,9 @@ 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 preConvertedArgs[i] != "":
// Use the pre-converted variable (conversion happened before PyEval_SaveThread).
na = preConvertedArgs[i]
case arg.sym.py2go != "":
na = fmt.Sprintf("%s(%s)%s", arg.sym.py2go, anm, arg.sym.py2goParenEx)
default:
Expand Down Expand Up @@ -415,7 +470,8 @@ if __err != nil {

if rvIsErr || nres == 2 {
g.gofile.Printf("\n")
// reacquire GIL
// release mutex then reacquire GIL
g.gofile.Printf("C.gopy_unlock()\n")
g.gofile.Printf("C.PyEval_RestoreThread(_saved_thread)\n")

g.gofile.Printf("if __err != nil {\n")
Expand Down
6 changes: 5 additions & 1 deletion bind/gen_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion bind/gen_slice.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion bind/gen_struct.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading
Loading