From ef7b4bc3ae7e9370ed38102aba29085f02ebbe8e Mon Sep 17 00:00:00 2001 From: b-long Date: Thu, 16 Apr 2026 23:53:16 +0000 Subject: [PATCH 01/12] WIP --- _examples/cgo/cgo.go | 2 +- bind/gen_func.go | 31 +++++++++++++++++++++++-------- bind/gen_map.go | 6 +++++- bind/gen_slice.go | 6 +++++- bind/gen_struct.go | 6 +++++- go.sum | 4 ++++ main_test.go | 10 ++++++++-- 7 files changed, 51 insertions(+), 14 deletions(-) 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/go.sum b/go.sum index 4b74f8cf..9f9cbc57 100644 --- a/go.sum +++ b/go.sum @@ -6,9 +6,13 @@ github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4= golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY= +golang.org/x/net v0.34.0/go.mod h1:di0qlW3YNM5oh6GqDGQr92MyTozJPmybPK4Ev/Gm31k= golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ= golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/sys v0.29.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/telemetry v0.0.0-20240521205824-bda55230c457/go.mod h1:pRgIJT+bRLFKnoM1ldnzKoxTIn14Yxz928LQRYYgIN0= golang.org/x/tools v0.29.0 h1:Xx0h3TtM9rzQpQuR4dKLrdglAmCEN5Oi+P74JdhdzXE= golang.org/x/tools v0.29.0/go.mod h1:KMQVMRsVxU6nHCFXrBPhDB8XncLNLM0lIy/F14RP588= diff --git a/main_test.go b/main_test.go index c5f6c29e..87ebfb07 100644 --- a/main_test.go +++ b/main_test.go @@ -12,6 +12,7 @@ import ( "os/exec" "path/filepath" "reflect" + "runtime" "sort" "strings" "testing" @@ -316,7 +317,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 +546,13 @@ 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)") + // Skip for some Go versions due to CGO issue + // See: https://github.com/go-python/gopy/issues/370 + if strings.HasPrefix(runtime.Version(), "go1.21") || + strings.HasPrefix(runtime.Version(), "go1.23") || + strings.HasPrefix(runtime.Version(), "go1.24") { + t.Skip("Skipping due to CGO issue (see https://github.com/go-python/gopy/issues/370)") + } // t.Parallel() path := "_examples/cgo" testPkg(t, pkg{ From c91ad4c5f45b6a56988e49c82e4eafa4023593ec Mon Sep 17 00:00:00 2001 From: b-long Date: Thu, 16 Apr 2026 21:25:11 -0400 Subject: [PATCH 02/12] expand test coverage Skip test for specific Go version due to CGO issue. --- main_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/main_test.go b/main_test.go index 87ebfb07..896ad120 100644 --- a/main_test.go +++ b/main_test.go @@ -548,9 +548,7 @@ OK func TestBindCgoPackage(t *testing.T) { // Skip for some Go versions due to CGO issue // See: https://github.com/go-python/gopy/issues/370 - if strings.HasPrefix(runtime.Version(), "go1.21") || - strings.HasPrefix(runtime.Version(), "go1.23") || - strings.HasPrefix(runtime.Version(), "go1.24") { + if strings.HasPrefix(runtime.Version(), "go1.23") { t.Skip("Skipping due to CGO issue (see https://github.com/go-python/gopy/issues/370)") } // t.Parallel() From 4848127e21b14e61b099af0409ba5b324f9a56cb Mon Sep 17 00:00:00 2001 From: b-long Date: Thu, 30 Apr 2026 06:15:36 -0400 Subject: [PATCH 03/12] remove skip in TestBindCgoPackage & fix CI matrix (#388) * remove skip condition for Go 1.23 in TestBindCgoPackage * remove skip condition for Go 1.23 in TestBindCgoPackage * update Go version matrix in CI configuration --- .github/workflows/ci.yml | 2 +- main_test.go | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) 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/main_test.go b/main_test.go index 896ad120..e7d4292c 100644 --- a/main_test.go +++ b/main_test.go @@ -12,7 +12,6 @@ import ( "os/exec" "path/filepath" "reflect" - "runtime" "sort" "strings" "testing" @@ -546,11 +545,6 @@ OK } func TestBindCgoPackage(t *testing.T) { - // Skip for some Go versions due to CGO issue - // See: https://github.com/go-python/gopy/issues/370 - if strings.HasPrefix(runtime.Version(), "go1.23") { - t.Skip("Skipping due to CGO issue (see https://github.com/go-python/gopy/issues/370)") - } // t.Parallel() path := "_examples/cgo" testPkg(t, pkg{ From f1f2ad0ae8c57bd47d08cb4534daa2a11e07e9d9 Mon Sep 17 00:00:00 2001 From: b-long Date: Thu, 30 Apr 2026 10:35:06 +0000 Subject: [PATCH 04/12] add regression tests for GIL ordering bug (issue #370) Adds two reproducers that exercise the go2py/C.CString-without-GIL crash: 1. A 5000-iteration stress loop in the cgo example (Hi/Hello string returns). 2. A new gilstring example covering struct string fields, slice elements, and map values under repeated calls. --- SUPPORT_MATRIX.md | 1 + _examples/cgo/test.py | 8 ++++++++ _examples/gilstring/gilstring.go | 27 +++++++++++++++++++++++++++ _examples/gilstring/test.py | 27 +++++++++++++++++++++++++++ main_test.go | 14 ++++++++++++++ 5 files changed, 77 insertions(+) create mode 100644 _examples/gilstring/gilstring.go create mode 100644 _examples/gilstring/test.py diff --git a/SUPPORT_MATRIX.md b/SUPPORT_MATRIX.md index 8e73c1ae..8f30be77 100644 --- a/SUPPORT_MATRIX.md +++ b/SUPPORT_MATRIX.md @@ -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 diff --git a/_examples/cgo/test.py b/_examples/cgo/test.py index 3fd4e635..51b418ea 100644 --- a/_examples/cgo/test.py +++ b/_examples/cgo/test.py @@ -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") diff --git a/_examples/gilstring/gilstring.go b/_examples/gilstring/gilstring.go new file mode 100644 index 00000000..c934c010 --- /dev/null +++ b/_examples/gilstring/gilstring.go @@ -0,0 +1,27 @@ +// 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 exercises go2py string conversion through functions, struct fields, +// slice elements, and map values — all of which previously ran C.CString +// without holding the GIL, causing crashes under repeated calls. +package gilstring + +// Item is a struct with a string field to exercise struct member string getters. +type Item struct { + Label string + Count int +} + +// MakeItem returns an Item with the given label. +func MakeItem(s string) Item { return Item{Label: s, Count: len(s)} } + +// GetLabel returns the Label field of an Item. +func GetLabel(i Item) string { return i.Label } + +// StringSlice returns a slice of strings. +func StringSlice() []string { return []string{"alpha", "beta", "gamma"} } + +// StringMap returns a map with string values. +func StringMap() map[string]string { return map[string]string{"key": "value"} } diff --git a/_examples/gilstring/test.py b/_examples/gilstring/test.py new file mode 100644 index 00000000..1193e029 --- /dev/null +++ b/_examples/gilstring/test.py @@ -0,0 +1,27 @@ +# 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 + +import gilstring + +# Regression test for GIL ordering bug (issue #370 / PR #386). +# Struct string fields, slice elements, and map values all previously used +# C.CString (go2py) without the GIL held, causing crashes under repeated calls. +N = 5000 + +for _ in range(N): + item = gilstring.MakeItem("hello") + assert item.Label == "hello", item.Label + +for _ in range(N): + s = gilstring.StringSlice() + assert s[0] == "alpha", s[0] + +for _ in range(N): + m = gilstring.StringMap() + assert m["key"] == "value", m["key"] + +print("OK") diff --git a/main_test.go b/main_test.go index e7d4292c..5f496fb8 100644 --- a/main_test.go +++ b/main_test.go @@ -49,6 +49,7 @@ var ( "_examples/cstrings": []string{"py3"}, "_examples/pkgconflict": []string{"py3"}, "_examples/variadic": []string{"py3"}, + "_examples/gilstring": []string{"py3"}, } testEnvironment = os.Environ() @@ -555,6 +556,7 @@ func TestBindCgoPackage(t *testing.T) { want: []byte(`cgo.doc: '\nPackage cgo tests bindings of CGo-based packages.\n\n' cgo.Hi()= 'hi from go\n' cgo.Hello(you)= 'hello you from go\n' +stress OK OK `), }) @@ -783,6 +785,18 @@ OK }) } +func TestGilString(t *testing.T) { + // t.Parallel() + path := "_examples/gilstring" + testPkg(t, pkg{ + path: path, + lang: features[path], + cmd: "build", + extras: nil, + want: []byte("OK\n"), + }) +} + func TestPackagePrefix(t *testing.T) { // t.Parallel() path := "_examples/package/mypkg" From 35ca29f734a2b7f15ea6f4b278f927995d0a1e74 Mon Sep 17 00:00:00 2001 From: b-long Date: Thu, 30 Apr 2026 23:21:43 +0000 Subject: [PATCH 05/12] expand test to mirror issue #370 reproduction --- _examples/gilstring/gilstring.go | 11 +++++++++++ _examples/gilstring/test.py | 14 ++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/_examples/gilstring/gilstring.go b/_examples/gilstring/gilstring.go index c934c010..90fb0db5 100644 --- a/_examples/gilstring/gilstring.go +++ b/_examples/gilstring/gilstring.go @@ -8,6 +8,17 @@ // without holding the GIL, causing crashes under repeated calls. package gilstring +import "fmt" + +// Add returns the sum of its arguments, mirroring simple.Add from the issue +// report reproduction script. +func Add(i, j int) int { return i + j } + +// Hello returns a greeting string, mirroring hi.Hello from the issue report +// reproduction script. Returning a non-trivial string stresses go2py +// string conversion (C.CString) on every call. +func Hello(s string) string { return fmt.Sprintf("Hello, %s!", s) } + // Item is a struct with a string field to exercise struct member string getters. type Item struct { Label string diff --git a/_examples/gilstring/test.py b/_examples/gilstring/test.py index 1193e029..1b0b56b2 100644 --- a/_examples/gilstring/test.py +++ b/_examples/gilstring/test.py @@ -8,10 +8,20 @@ import gilstring # Regression test for GIL ordering bug (issue #370 / PR #386). -# Struct string fields, slice elements, and map values all previously used -# C.CString (go2py) without the GIL held, causing crashes under repeated calls. +# Mirrors the exact reproduction script from the issue report: +# for _ in range(5000): +# Add(2, 2) +# Hello('hi') +# Integer arithmetic (Add) interleaved with string-returning calls (Hello) +# stresses the go2py C.CString path that previously ran without the GIL held. N = 5000 +for _ in range(N): + assert gilstring.Add(2, 2) == 4 + assert gilstring.Hello("hi") == "Hello, hi!" + +# Struct string fields, slice elements, and map values exercise additional +# go2py string conversion paths from the same bug. for _ in range(N): item = gilstring.MakeItem("hello") assert item.Label == "hello", item.Label From aea9204f912348049da365791e4f3c6e0842d9b6 Mon Sep 17 00:00:00 2001 From: b-long Date: Sat, 2 May 2026 01:44:41 +0000 Subject: [PATCH 06/12] Update reproduction of crash on x86_64: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - gilstring.go reduced to a single Hello() function (mirrors hi.Hello from the issue report) - test.py imports both gilstring and simple as two separately-built extensions in the same Python process, interleaving Add/Hello calls over 5000 iterations - TestGilString builds each package into its own subdir to prevent C symbol collisions, then runs test.py with a shared PYTHONPATH root - ci.yml adds macos-15-intel (x86_64) to the matrix — the platform where "fatal error: bad sweepgen in refill" reliably reproduces --- .github/workflows/ci.yml | 19 +++++++- _examples/gilstring/gilstring.go | 32 ++----------- _examples/gilstring/test.py | 37 ++++---------- main_test.go | 82 ++++++++++++++++++++++++++++---- 4 files changed, 104 insertions(+), 66 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 79cca444..ca7d7d25 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,7 +23,7 @@ jobs: fail-fast: false matrix: 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, windows-latest, macos-15-intel] #platform: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.platform }} steps: @@ -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' @@ -77,6 +84,16 @@ 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: Upload-Coverage if: matrix.platform == 'ubuntu-latest' uses: codecov/codecov-action@v4 diff --git a/_examples/gilstring/gilstring.go b/_examples/gilstring/gilstring.go index 90fb0db5..df160282 100644 --- a/_examples/gilstring/gilstring.go +++ b/_examples/gilstring/gilstring.go @@ -3,36 +3,12 @@ // license that can be found in the LICENSE file. // Package gilstring is a regression test for the GIL ordering bug (issue #370). -// It exercises go2py string conversion through functions, struct fields, -// slice elements, and map values — all of which previously ran C.CString -// without holding the GIL, causing crashes under repeated calls. +// 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" -// Add returns the sum of its arguments, mirroring simple.Add from the issue -// report reproduction script. -func Add(i, j int) int { return i + j } - -// Hello returns a greeting string, mirroring hi.Hello from the issue report -// reproduction script. Returning a non-trivial string stresses go2py -// string conversion (C.CString) on every call. +// Hello returns a greeting string, mirroring hi.Hello from the issue report. func Hello(s string) string { return fmt.Sprintf("Hello, %s!", s) } - -// Item is a struct with a string field to exercise struct member string getters. -type Item struct { - Label string - Count int -} - -// MakeItem returns an Item with the given label. -func MakeItem(s string) Item { return Item{Label: s, Count: len(s)} } - -// GetLabel returns the Label field of an Item. -func GetLabel(i Item) string { return i.Label } - -// StringSlice returns a slice of strings. -func StringSlice() []string { return []string{"alpha", "beta", "gamma"} } - -// StringMap returns a map with string values. -func StringMap() map[string]string { return map[string]string{"key": "value"} } diff --git a/_examples/gilstring/test.py b/_examples/gilstring/test.py index 1b0b56b2..e4c14818 100644 --- a/_examples/gilstring/test.py +++ b/_examples/gilstring/test.py @@ -5,33 +5,14 @@ ## py2/py3 compat from __future__ import print_function -import gilstring - -# Regression test for GIL ordering bug (issue #370 / PR #386). -# Mirrors the exact reproduction script from the issue report: -# for _ in range(5000): -# Add(2, 2) -# Hello('hi') -# Integer arithmetic (Add) interleaved with string-returning calls (Hello) -# stresses the go2py C.CString path that previously ran without the GIL held. -N = 5000 - -for _ in range(N): - assert gilstring.Add(2, 2) == 4 - assert gilstring.Hello("hi") == "Hello, hi!" - -# Struct string fields, slice elements, and map values exercise additional -# go2py string conversion paths from the same bug. -for _ in range(N): - item = gilstring.MakeItem("hello") - assert item.Label == "hello", item.Label - -for _ in range(N): - s = gilstring.StringSlice() - assert s[0] == "alpha", s[0] - -for _ in range(N): - m = gilstring.StringMap() - assert m["key"] == "value", m["key"] +# 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") diff --git a/main_test.go b/main_test.go index 5f496fb8..bbfb23ec 100644 --- a/main_test.go +++ b/main_test.go @@ -785,16 +785,80 @@ OK }) } +// TestGilString is a regression test for the GIL ordering bug (issue #370). +// It replicates the exact reproduction from the issue report: two separately-built +// gopy extensions (gilstring and simple) loaded in the same Python process, with +// calls interleaved in a loop of 5000 iterations. func TestGilString(t *testing.T) { - // t.Parallel() - path := "_examples/gilstring" - testPkg(t, pkg{ - path: path, - lang: features[path], - cmd: "build", - extras: nil, - want: []byte("OK\n"), - }) + backends := []string{"py3"} + for _, be := range backends { + vm, ok := testBackends[be] + if !ok || vm == "" { + t.Logf("Skipped testing backend %s for TestGilString\n", be) + continue + } + t.Run(be, func(t *testing.T) { + cwd, _ := os.Getwd() + + // workdir is the PYTHONPATH root; each package gets its own subdir + // so their generated C files don't collide during compilation. + workdir, err := os.MkdirTemp("", "gopy-") + if err != nil { + t.Fatalf("could not create workdir: %v", err) + } + defer os.RemoveAll(workdir) + defer bind.ResetPackages() + + gilDir := filepath.Join(workdir, "gilstring") + if err := os.MkdirAll(gilDir, 0700); err != nil { + t.Fatalf("could not create gilstring subdir: %v", err) + } + + // Build gilstring into its own subdir. + writeGoMod(t, cwd, gilDir) + if err := run([]string{"build", "-vm=" + vm, "-output=" + gilDir, "./_examples/gilstring"}); err != nil { + t.Fatalf("error building gilstring: %v", err) + } + bind.ResetPackages() + + simpleDir := filepath.Join(workdir, "simple") + if err := os.MkdirAll(simpleDir, 0700); err != nil { + t.Fatalf("could not create simple subdir: %v", err) + } + + // Build simple into its own subdir. + writeGoMod(t, cwd, simpleDir) + if err := run([]string{"build", "-vm=" + vm, "-output=" + simpleDir, "./_examples/simple"}); err != nil { + t.Fatalf("error building simple: %v", err) + } + + // Copy test.py into workdir root and run with PYTHONPATH=workdir. + tstSrc := filepath.Join(cwd, "_examples/gilstring/test.py") + tstDst := filepath.Join(workdir, "test.py") + if err := copyCmd(tstSrc, tstDst); err != nil { + t.Fatalf("error copying test.py: %v", err) + } + + env := make([]string, len(testEnvironment)) + copy(env, testEnvironment) + env = append(env, fmt.Sprintf("PYTHONPATH=%s", workdir)) + + cmd := exec.Command(vm, "./test.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) { From 55ee3fa6c02fd7e3e3710b099a9c342a729fb56b Mon Sep 17 00:00:00 2001 From: b-long Date: Sat, 2 May 2026 02:08:44 +0000 Subject: [PATCH 07/12] =?UTF-8?q?fix:=20acquire=20GIL=20before=20C?= =?UTF-8?q?=E2=86=92Go=20arg=20conversion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C.GoString (and other py2go converters) call runtime.gostring → mallocgc inside a CGo callback. If the GIL is released before those conversions, Go's GC can observe a corrupted sweep-generation counter, causing "fatal error: bad sweepgen in refill" on Go ≥1.24 / macOS x86_64 (issue #370). In genFuncBody(), pre-convert each py2go argument into a local variable while the GIL is held via PyGILState_Ensure/Release, then release the GIL for the actual Go function call as before. The callArgs loop now references the pre-converted variable instead of inlining C.GoString() after SaveThread. Also documents Idea 2 (unsafe.String zero-alloc approach) as a future defence-in-depth option in a code comment. --- bind/gen_func.go | 78 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 23 deletions(-) diff --git a/bind/gen_func.go b/bind/gen_func.go index 75aacfd7..8e6acc18 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -261,20 +261,53 @@ func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) { } } + // 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() { + 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() { + 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") + } + // release GIL g.gofile.Printf("_saved_thread := C.PyEval_SaveThread()\n") - // 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 { + if !rvIsErr && nres != 2 { // reacquire GIL after return g.gofile.Printf("defer C.PyEval_RestoreThread(_saved_thread)\n") } @@ -317,6 +350,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: @@ -368,8 +404,8 @@ if __err != nil { g.pywrap.Printf("_%s.%s(", pkgname, mnm) } - hasAddrOfTmp := false hasRetCvt := false + hasAddrOfTmp := false if nres > 0 { ret := res[0] switch { @@ -381,10 +417,8 @@ 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("cret := ") + g.gofile.Printf("return %s(", ret.sym.go2py) default: g.gofile.Printf("return ") } @@ -407,6 +441,11 @@ 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() @@ -421,13 +460,6 @@ 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 From 8740cc3fba0c14a501f59237a17c35b978869150 Mon Sep 17 00:00:00 2001 From: b-long Date: Sat, 2 May 2026 02:27:38 +0000 Subject: [PATCH 08/12] =?UTF-8?q?fix:=20acquire=20GIL=20before=20C?= =?UTF-8?q?=E2=86=92Go=20argument=20conversion=20in=20generated=20CGo=20wr?= =?UTF-8?q?appers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C.GoString (and other py2go converters) call runtime.gostring → mallocgc inside a CGo callback. If those conversions run after PyEval_SaveThread releases the GIL, Go's GC can observe a corrupted sweep-generation counter, causing "fatal error: bad sweepgen in refill" on Go ≥1.24 / macOS x86_64 (issue #370). In genFuncBody(), pre-convert each py2go argument into a local variable while the GIL is held via PyGILState_Ensure/Release, then release the GIL for the actual Go function call as before. Interface-handle arguments (ifchandle && goname == "interface{}") are excluded from pre-conversion, matching the existing callArgs switch logic to avoid type mismatches in generated code for the iface example. Also documents Idea 2 (unsafe.String zero-alloc approach) as a future defence-in-depth option in a code comment. --- bind/gen_func.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bind/gen_func.go b/bind/gen_func.go index 8e6acc18..06031c39 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -287,7 +287,7 @@ func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) { preConvertedArgs := map[int]string{} // arg index → temp variable name needsGILForArgs := false for _, arg := range args { - if arg.sym.py2go != "" && !arg.sym.isSignature() { + if arg.sym.py2go != "" && !arg.sym.isSignature() && !(ifchandle && arg.sym.goname == "interface{}") { needsGILForArgs = true break } @@ -295,7 +295,7 @@ func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) { if needsGILForArgs { g.gofile.Printf("_gstate := C.PyGILState_Ensure()\n") for i, arg := range args { - if arg.sym.py2go != "" && !arg.sym.isSignature() { + 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) From 029931136bdaf822279cb5392120ea2c6ec8bd45 Mon Sep 17 00:00:00 2001 From: b-long Date: Sat, 2 May 2026 14:32:18 +0000 Subject: [PATCH 09/12] ci: add symbol diagnostic step for macOS Intel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On macos-15-intel, two separately-built gopy extensions loaded in the same Python process can crash with "fatal error: bad sweepgen in refill" on certain Go versions. The root cause is not yet confirmed: candidate mechanisms include PLT-based CGo symbol interposition (crosscall2, _cgo_topofstack, x_cgo_inittls, etc.) and/or dyld global-namespace deduplication of the ~150 runtime symbols exported by both .so files. Add a diagnostic step that runs on every macos-15-intel job (pass or fail) and reports three things: 1. how many dynamic symbols are shared between the two extensions 2. which of the critical CGo bridge symbols appear in the indirect symbol table (otool -Iv) — the macOS equivalent of JUMP_SLOT/PLT 3. which library wins in the global namespace at runtime (ctypes) Comparing the output across Go 1.21/1.22 (fail), 1.23/1.24 (pass), and 1.25 (fail) should confirm whether the crash correlates with PLT stub generation changes between Go versions. --- .github/workflows/ci.yml | 69 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ca7d7d25..62f52149 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -94,6 +94,75 @@ jobs: 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 From 2f3f1ba2bb38cdad5b5d398497ffefb94e712bc6 Mon Sep 17 00:00:00 2001 From: b-long Date: Sun, 3 May 2026 01:41:57 +0000 Subject: [PATCH 10/12] fix: serialize cross-extension Go calls via a process-wide pthread mutex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Loading two gopy extensions in the same Python process embeds two independent Go runtimes. On macOS x86_64 / Go ≥1.24 this causes "fatal error: bad sweepgen in refill" (issue #370) when both runtimes run Go code concurrently. Add a process-wide pthread_mutex_t stored as a Python capsule in builtins._gopy_global_mu so every gopy extension in the same interpreter shares the same lock. The generated CGo wrappers: 1. Call gopy_ensure_mu() (lazy init, Python GIL must be held) before releasing the GIL. 2. Release the GIL via PyEval_SaveThread. 3. Acquire the mutex via gopy_lock() — blocking until any other extension's Go call finishes. 4. Release the mutex (gopy_unlock()) before restoring the GIL (PyEval_RestoreThread), avoiding the GIL/mutex deadlock. On Windows the lock/unlock are compiled as no-ops. Fixes #370 / #385. --- bind/gen.go | 36 ++++++++++++++++++++++++++++++++++++ bind/gen_func.go | 15 ++++++++++++--- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/bind/gen.go b/bind/gen.go index 3685bbab..7bf7195f 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -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 +#include +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" diff --git a/bind/gen_func.go b/bind/gen_func.go index 06031c39..9f13bbc8 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -305,11 +305,19 @@ func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) { g.gofile.Printf("C.PyGILState_Release(_gstate)\n") } - // release GIL + // 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 { @@ -462,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") From 5072ba05cb727d8d98ee1723539d828a3ceed92e Mon Sep 17 00:00:00 2001 From: b-long Date: Sun, 3 May 2026 02:13:05 +0000 Subject: [PATCH 11/12] fix: restrict exported symbols in final .so to prevent Go runtime interposition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When two gopy extensions are loaded in the same Python process via RTLD_GLOBAL, Go runtime data globals (mcache0, allm, mheap_, etc.) from the first-loaded library win in the dynamic-linker global namespace. The second runtime's references to those globals are silently redirected, so both runtimes share the same heap metadata. This corrupts sweep- generation counters and causes: fatal error: bad sweepgen in refill on macOS x86_64 / Go ≥1.24 (the earlier pthread mutex fix serialised user code but could not stop background GC goroutines that also hit the shared globals). Fix: pass a symbol-visibility restriction to the final go build step so that only PyInit__ is exported into the global namespace: - macOS: -extldflags=-Wl,-exported_symbols_list, - Linux: -extldflags=-Wl,--version-script, All CGo bridge symbols (crosscall2, _cgo_topofstack, …) remain in the .so and are called directly at link time; they no longer pollute the global namespace and cannot be interposed by a second extension. Fixes #370 / #385. --- cmd_build.go | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/cmd_build.go b/cmd_build.go index 24b0191c..251c71f7 100644 --- a/cmd_build.go +++ b/cmd_build.go @@ -185,12 +185,15 @@ func runBuild(mode bind.BuildMode, cfg *BuildCfg) error { if cfg.BuildTags != "" { args = append(args, "-tags", cfg.BuildTags) } + // Collect all Go linker flags into one -ldflags argument so we can + // combine -s/-w with the symbol-visibility extldflags below. + var goLdFlags []string if !cfg.Symbols { // These flags will omit the various symbol tables, thereby // reducing the final size of the binary. From https://golang.org/cmd/link/ // -s Omit the symbol table and debug information // -w Omit the DWARF symbol table - args = append(args, "-ldflags=-s -w") + goLdFlags = append(goLdFlags, "-s", "-w") } args = append(args, "-o", buildLib, ".") fmt.Printf("go %v\n", strings.Join(args, " ")) @@ -214,6 +217,42 @@ func runBuild(mode bind.BuildMode, cfg *BuildCfg) error { return err } + // Restrict exported symbols to only PyInit__ so that Go runtime + // globals (e.g. runtime.mheap_, mcache0) are not placed in the global + // dynamic-linker namespace. Two independently-loaded Go runtimes + // sharing those globals via RTLD_GLOBAL interposition corrupt each + // other's GC sweep-generation counters (issue #370 / #385). + switch runtime.GOOS { + case "darwin": + // macOS exported_symbols_list: one symbol per line, with leading _. + ef, ferr := os.CreateTemp("", "gopy-exports-*.txt") + if ferr == nil { + fmt.Fprintf(ef, "_%sPyInit__%s\n", "", cfg.Name) + ef.Close() + defer os.Remove(ef.Name()) + goLdFlags = append(goLdFlags, + "-extldflags=-Wl,-exported_symbols_list,"+ef.Name()) + } + case "linux": + // GNU ld version script: export only PyInit__. + ef, ferr := os.CreateTemp("", "gopy-exports-*.map") + if ferr == nil { + fmt.Fprintf(ef, "{ global: PyInit__%s; local: *; };\n", cfg.Name) + ef.Close() + defer os.Remove(ef.Name()) + goLdFlags = append(goLdFlags, + "-extldflags=-Wl,--version-script="+ef.Name()) + } + } + if len(goLdFlags) > 0 { + // Replace or append the single combined -ldflags argument. + // The args slice currently ends with "-o", modlib, "." + // Insert before the final ".". + combined := "-ldflags=" + strings.Join(goLdFlags, " ") + n := len(args) + args = append(args[:n-1], combined, args[n-1]) + } + if bind.WindowsOS { fmt.Printf("Doing windows sed hack to fix declspec for PyInit\n") fname := cfg.Name + ".c" From dd6afb9828c789719e89a3b96cc41d39895d0e3f Mon Sep 17 00:00:00 2001 From: b-long Date: Sun, 3 May 2026 02:35:04 +0000 Subject: [PATCH 12/12] fix: load gopy extensions with RTLD_LOCAL to prevent Go runtime interposition On macOS, Python's default dlopen flags are RTLD_NOW|RTLD_GLOBAL (Py_RTLD_DEFAULT in configure.ac for Darwin). Every .so extension imported via the normal Python import machinery is therefore loaded into the process-wide flat namespace. When two gopy extensions are loaded in the same process, the second extension's Go runtime symbols (TLS keys, mheap_, cgo init pointers) get interposed by the first extension's definitions, causing the two independent Go runtimes to share GC state and triggering 'fatal error: bad sweepgen in refill' (issue #385). The generated Python wrapper now temporarily clears RTLD_GLOBAL before importing the underlying _.so, so each extension's Go runtime keeps its own isolated copy of these globals. The original flags are restored immediately after import so the rest of the program is unaffected. --- bind/gen.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/bind/gen.go b/bind/gen.go index 7bf7195f..65617e9f 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -317,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: