From de1757e127b3ee82b94780072fb7d13a51b958c8 Mon Sep 17 00:00:00 2001 From: Ben Carver Date: Sat, 20 Jul 2024 14:20:30 +0000 Subject: [PATCH 1/5] Resolved issue; need to acquire GIL before calling C.PyCallable_Check --- bind/symbols.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/bind/symbols.go b/bind/symbols.go index 0ddc8c73..28349f08 100644 --- a/bind/symbols.go +++ b/bind/symbols.go @@ -1083,24 +1083,32 @@ func (sym *symtab) addSignatureType(pkg *types.Package, obj types.Object, t type py2g := fmt.Sprintf("%s { ", nsig) + py2g += "_gstate := C.PyGILState_Ensure()\n" + // TODO: use strings.Builder if rets.Len() == 0 { - py2g += "if C.PyCallable_Check(_fun_arg) == 0 { return }\n" + py2g += "if C.PyCallable_Check(_fun_arg) == 0 {\n" + py2g += "C.PyGILState_Release(_gstate)\n" + py2g += "return\n" + py2g += "}" } else { zstr, err := sym.ZeroToGo(ret.Type(), rsym) if err != nil { return err } - py2g += fmt.Sprintf("if C.PyCallable_Check(_fun_arg) == 0 { return %s }\n", zstr) + py2g += "if C.PyCallable_Check(_fun_arg) == 0 {\n" + py2g += "C.PyGILState_Release(_gstate)\n" + py2g += fmt.Sprintf("return %s\n", zstr) + py2g += "}" } - py2g += "_gstate := C.PyGILState_Ensure()\n" + if nargs > 0 { bstr, err := sym.buildTuple(args, "_fcargs", "_fun_arg") if err != nil { return err } py2g += bstr + retstr - py2g += fmt.Sprintf("C.PyObject_CallObject(_fun_arg, _fcargs)\n") + py2g += "C.PyObject_CallObject(_fun_arg, _fcargs)\n" py2g += "C.gopy_decref(_fcargs)\n" } else { // TODO: methods not supported for no-args case -- requires self arg.. From 1aba0d57e231799d11f313dda3fca3ed916f91c6 Mon Sep 17 00:00:00 2001 From: Ben Carver Date: Sat, 20 Jul 2024 14:54:56 +0000 Subject: [PATCH 2/5] Added newline character to closing bracket --- bind/symbols.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bind/symbols.go b/bind/symbols.go index 28349f08..67bb6a5b 100644 --- a/bind/symbols.go +++ b/bind/symbols.go @@ -1088,18 +1088,18 @@ func (sym *symtab) addSignatureType(pkg *types.Package, obj types.Object, t type // TODO: use strings.Builder if rets.Len() == 0 { py2g += "if C.PyCallable_Check(_fun_arg) == 0 {\n" - py2g += "C.PyGILState_Release(_gstate)\n" + py2g += "C.PyGILState_Release(_gstate)\n" // Release GIL py2g += "return\n" - py2g += "}" + py2g += "}\n" } else { zstr, err := sym.ZeroToGo(ret.Type(), rsym) if err != nil { return err } py2g += "if C.PyCallable_Check(_fun_arg) == 0 {\n" - py2g += "C.PyGILState_Release(_gstate)\n" + py2g += "C.PyGILState_Release(_gstate)\n" // Release GIL py2g += fmt.Sprintf("return %s\n", zstr) - py2g += "}" + py2g += "}\n" } if nargs > 0 { From 9d692d439d89e2e1d7a3799366e3dcb56bcdf88c Mon Sep 17 00:00:00 2001 From: b-long Date: Mon, 4 May 2026 14:35:29 -0400 Subject: [PATCH 3/5] Fix memory leaks and crashes when loading multiple Go extensions in one Python process Several related bugs caused TestCStrings to fail and could crash programs that import more than one gopy-built package at the same time: 1. C string leak in generated getters. When reading a string field from a Go struct, slice, or map, the generated code allocated a C string with C.CString() but never freed it. Over thousands of calls this accumulated enough memory to exceed the leak threshold in TestCStrings. Fixed by switching those accessors to use add_checked_string_function, which adds the missing free() after the Python string is built. 2. Unsafe Go GC call from a CGo thread. The generated wrapper called runtime.GC() directly while Python's garbage collector was running, which could panic with "bad sweepgen in refill" on multi-core machines. Fixed by routing the GC call through a background goroutine via a channel, so it always runs on a proper Go thread. 3. Go GC and Python GC out of sync. Without any coordination, Go heap objects could pile up between Python GC cycles, causing RSS to grow across test passes. The generated wrapper now registers a Python gc.callbacks handler that triggers a Go GC cycle after each Python GC cycle, keeping the two runtimes in sync automatically. 4. Symbol interposition between co-loaded extensions. On some platforms, loading two gopy .so files with RTLD_GLOBAL caused Go runtime symbols from one extension to override those in the other, corrupting heap state. Fixed by loading extensions without RTLD_GLOBAL and clearing the Go TLS slot before each CGo entry so each extension uses its own runtime context. Also adds macos-15 (ARM) and macos-15-intel to the CI matrix, and includes a new gilstring regression test that exercises two extensions in one process. --- .github/workflows/ci.yml | 17 ++++++- SUPPORT_MATRIX.md | 1 + _examples/cgo/cgo.go | 2 +- _examples/gilstring/gilstring.go | 14 ++++++ _examples/gilstring/test.py | 18 ++++++++ bind/gen.go | 78 ++++++++++++++++++++++++++++++++ bind/gen_func.go | 9 ++-- bind/gen_map.go | 6 ++- bind/gen_slice.go | 6 ++- bind/gen_struct.go | 6 ++- cmd_build.go | 53 ++++++++++++++++++---- go.sum | 4 ++ main_test.go | 72 +++++++++++++++++++++++++++-- 13 files changed, 267 insertions(+), 19 deletions(-) create mode 100644 _examples/gilstring/gilstring.go create mode 100644 _examples/gilstring/test.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e232aa0c..3684d0b7 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.22.x, 1.21.x] - platform: [ubuntu-latest, windows-latest] + platform: [ubuntu-latest, windows-latest, macos-15-intel, macos-15] #platform: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.platform }} steps: @@ -51,6 +51,12 @@ jobs: # install goimports go install golang.org/x/tools/cmd/goimports@latest + - name: Install macOS packages + if: startsWith(matrix.platform, 'macos-') + run: | + python3 -m pip install -U pybindgen + go install golang.org/x/tools/cmd/goimports@latest + - name: Install Windows packages if: matrix.platform == 'windows-latest' run: | @@ -69,6 +75,15 @@ jobs: run: | make test + - name: Build-macOS + if: startsWith(matrix.platform, 'macos-') + run: | + make + - name: Test macOS + if: startsWith(matrix.platform, 'macos-') + run: | + make test + - name: Build-Windows if: matrix.platform == 'windows-latest' run: | 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/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/_examples/gilstring/gilstring.go b/_examples/gilstring/gilstring.go new file mode 100644 index 00000000..9410c57a --- /dev/null +++ b/_examples/gilstring/gilstring.go @@ -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 multi-runtime crash (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) } diff --git a/_examples/gilstring/test.py b/_examples/gilstring/test.py new file mode 100644 index 00000000..1aae7b31 --- /dev/null +++ b/_examples/gilstring/test.py @@ -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 multi-runtime crash (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/bind/gen.go b/bind/gen.go index 3685bbab..97c1ade6 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -87,10 +87,27 @@ static inline void gopy_err_handle() { PyErr_Print(); } } +// _gopy_clear_go_tls clears the Go goroutine pointer from this thread's TLS. +// When multiple gopy extensions share a process, each has its own Go runtime +// but all runtimes use the same TLS slot for the current goroutine pointer +// (GS:0x30 on darwin/amd64, FS:-8 on linux/amd64). After one extension's +// init(), TLS is left pointing to that runtime's g0. If another extension's +// CGo entry-point reads TLS and finds a non-nil goroutine, it takes the fast +// path (no needm()) and runs with the wrong M/P/mcache -- corrupting the heap. +// Clearing the slot before each CGo entry forces needm() to run, which +// establishes the correct per-extension context (issue #370). +static void _gopy_clear_go_tls(void) { +#if defined(__x86_64__) && defined(__APPLE__) + __asm__ volatile("movq $0, %%%%gs:0x30" ::: "memory"); +#elif defined(__x86_64__) && defined(__linux__) + __asm__ volatile("movq $0, %%%%fs:-8" ::: "memory"); +#endif +} %[8]s */ import "C" import ( + "runtime" "github.com/go-python/gopy/gopyh" // handler %[6]s ) @@ -132,6 +149,34 @@ func NumHandles() int { return gopyh.NumHandles() } +// _gcReq carries GC requests from RequestGC (called on a CGo/needm M) to a +// dedicated goroutine that actually calls runtime.GC(). Calling runtime.GC() +// directly from the gc.callbacks context (a CGo-needm M) races with goroutines +// mid-sweep on the same heap, causing "bad sweepgen in refill" panics on +// multi-core machines. Running GC on a proper goroutine eliminates that race. +// RequestGC blocks until the GC cycle completes, so Python memory measurements +// taken immediately after gc.collect() see the reclaimed Go memory. +var _gcReq = make(chan chan struct{}) + +func init() { + go func() { + for done := range _gcReq { + runtime.GC() + close(done) + } + }() +} + +// RequestGC runs Go's garbage collector synchronously and safely. +// gopy registers this via Python gc.callbacks so it fires after each Python +// GC cycle, keeping Go-heap objects freed via DecRef promptly collected. +//export RequestGC +func RequestGC() { + done := make(chan struct{}) + _gcReq <- done + <-done +} + // boolGoToPy converts a Go bool to python-compatible C.char func boolGoToPy(b bool) C.char { if b { @@ -259,6 +304,8 @@ mod.add_function('GoPyInit', None, []) mod.add_function('DecRef', None, [param('int64_t', 'handle')]) mod.add_function('IncRef', None, [param('int64_t', 'handle')]) mod.add_function('NumHandles', retval('int'), []) +mod.add_function('RequestGC', None, []) +mod.add_function('_gopy_clear_go_tls', None, []) ` // appended to imports in py wrap preamble as key for adding at end @@ -281,8 +328,39 @@ except ImportError: cwd = os.getcwd() currentdir = os.path.dirname(os.path.abspath(inspect.getfile(inspect.currentframe()))) os.chdir(currentdir) +# When multiple gopy extensions coexist in one Python process each carries its own +# independent Go runtime. The Go extension is loaded without RTLD_GLOBAL below, and +# _gopy_clear_go_tls() is called before each CGo entry to force needm() to run, which +# establishes the correct per-extension M/P/mcache context (issue #370). +# Also load the extension without RTLD_GLOBAL so that Go runtime symbols stay +# local to each .so — belt-and-suspenders on platforms where RTLD_GLOBAL is the +# Python default (e.g. some Linux builds). +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) +# Run Go's GC whenever Python's GC runs so that Go-heap objects whose handles +# were released via DecRef are promptly collected. Without this, Go memory +# can accumulate between Python gc.collect() calls because Python GC only +# frees the Python wrapper; the underlying Go allocation is not reclaimed +# until Go's own GC fires. +try: + import gc as _gopy_gc + def _gopy_gc_cb(phase, info): + if phase == 'stop': + _%[1]s.RequestGC() + _gopy_gc.callbacks.append(_gopy_gc_cb) +except Exception: + pass # to use this code in your end-user python file, import it as follows: # from %[1]s import %[3]s diff --git a/bind/gen_func.go b/bind/gen_func.go index 8f2e606e..fc2ee15d 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -261,10 +261,8 @@ func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) { } } - // release GIL g.gofile.Printf("_saved_thread := C.PyEval_SaveThread()\n") if !rvIsErr && nres != 2 { - // reacquire GIL after return g.gofile.Printf("defer C.PyEval_RestoreThread(_saved_thread)\n") } @@ -338,6 +336,12 @@ if __err != nil { } } + // Clear the Go TLS goroutine slot before the CGo entry point so that + // Go's needm() runs and establishes the correct per-extension context. + // Without this, two extensions sharing the same process can corrupt + // each other's heap via TLS collision (issue #370). + g.pywrap.Printf("_%s._gopy_clear_go_tls()\n", pkgname) + // pywrap output mnm := fsym.ID() if isMethod { @@ -415,7 +419,6 @@ if __err != nil { if rvIsErr || nres == 2 { g.gofile.Printf("\n") - // reacquire GIL g.gofile.Printf("C.PyEval_RestoreThread(_saved_thread)\n") g.gofile.Printf("if __err != nil {\n") diff --git a/bind/gen_map.go b/bind/gen_map.go index f65111fa..06d3302d 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.cpyname == "char*" { + 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..1c16180c 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.cpyname == "char*" { + 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..076d09aa 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.cpyname == "char*" { + 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/cmd_build.go b/cmd_build.go index 24b0191c..3def118a 100644 --- a/cmd_build.go +++ b/cmd_build.go @@ -181,30 +181,67 @@ func runBuild(mode bind.BuildMode, cfg *BuildCfg) error { // build the go shared library upfront to generate the header // needed by our generated cpython code - args := []string{"build", "-mod=mod", "-buildmode=c-shared"} + firstArgs := []string{"build", "-mod=mod", "-buildmode=c-shared"} if cfg.BuildTags != "" { - args = append(args, "-tags", cfg.BuildTags) + firstArgs = append(firstArgs, "-tags", cfg.BuildTags) } 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") + firstArgs = append(firstArgs, "-ldflags=-s -w") } - args = append(args, "-o", buildLib, ".") - fmt.Printf("go %v\n", strings.Join(args, " ")) - cmd = exec.Command("go", args...) + firstArgs = append(firstArgs, "-o", buildLib, ".") + fmt.Printf("go %v\n", strings.Join(firstArgs, " ")) + cmd = exec.Command("go", firstArgs...) cmdout, err = cmd.CombinedOutput() if err != nil { fmt.Printf("cmd had error: %v output:\n%v\n", err, string(cmdout)) return err } - // update the output name to the one with the ABI extension - args[len(args)-2] = modlib // we don't need this initial lib because we are going to relink os.Remove(buildLib) + // Build the final extension with symbol-visibility restriction so that + // Go runtime globals 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 state (#370). + // This applies only to the second build, which is where PyInit__ + // exists and where the exported-symbols list is valid. + finalArgs := []string{"build", "-mod=mod", "-buildmode=c-shared"} + if cfg.BuildTags != "" { + finalArgs = append(finalArgs, "-tags", cfg.BuildTags) + } + var finalLdFlags []string + if !cfg.Symbols { + finalLdFlags = append(finalLdFlags, "-s", "-w") + } + switch runtime.GOOS { + case "darwin": + ef, ferr := os.CreateTemp("", "gopy-exports-*.txt") + if ferr == nil { + fmt.Fprintf(ef, "_PyInit__%s\n", cfg.Name) + ef.Close() + defer os.Remove(ef.Name()) + finalLdFlags = append(finalLdFlags, "-extldflags=-Wl,-exported_symbols_list,"+ef.Name()) + } + case "linux": + 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()) + finalLdFlags = append(finalLdFlags, "-extldflags=-Wl,--version-script="+ef.Name()) + } + } + if len(finalLdFlags) > 0 { + finalArgs = append(finalArgs, "-ldflags="+strings.Join(finalLdFlags, " ")) + } + finalArgs = append(finalArgs, "-o", modlib, ".") + // args is still used below for the CGO env build; point it at finalArgs. + args := finalArgs + // generate c code fmt.Printf("%v build.py\n", cfg.VM) cmd = exec.Command(cfg.VM, "build.py") 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..1127698c 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() @@ -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,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{ @@ -774,7 +773,6 @@ func TestCStrings(t *testing.T) { lang: features[path], cmd: "build", extras: nil, - // todo: this test on mac leaks everything except String want: []byte(`gofnString leaked: False gofnStruct leaked: False gofnNestedStruct leaked: False @@ -785,6 +783,74 @@ OK }) } +// TestGilString is a regression test for the multi-runtime crash (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) { + 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, 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) + } + 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) + } + writeGoMod(t, cwd, simpleDir) + if err := run([]string{"build", "-vm=" + vm, "-output=" + simpleDir, "./_examples/simple"}); err != nil { + t.Fatalf("error building simple: %v", err) + } + + tstDst := filepath.Join(workdir, "test.py") + if err := copyCmd(filepath.Join(cwd, "_examples/gilstring/test.py"), 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) { // t.Parallel() path := "_examples/package/mypkg" From 16d953b8f96fc9539dbf868749b063d9e28729dd Mon Sep 17 00:00:00 2001 From: b-long Date: Tue, 12 May 2026 11:04:58 +0000 Subject: [PATCH 4/5] ci: remove macos-15-intel from support matrix --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3684d0b7..81fe5514 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.22.x, 1.21.x] - platform: [ubuntu-latest, windows-latest, macos-15-intel, macos-15] + platform: [ubuntu-latest, windows-latest, macos-15] #platform: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.platform }} steps: From 322e1ce5ea37f6bd29d6bc28aabd907aa65d1c16 Mon Sep 17 00:00:00 2001 From: b-long Date: Tue, 12 May 2026 11:05:44 +0000 Subject: [PATCH 5/5] ci: add golang 1.23.x to support matrix --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 81fe5514..183a6498 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, macos-15] #platform: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.platform }}