From de1757e127b3ee82b94780072fb7d13a51b958c8 Mon Sep 17 00:00:00 2001 From: Ben Carver Date: Sat, 20 Jul 2024 14:20:30 +0000 Subject: [PATCH 01/10] 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 02/10] 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 0a48f89430a0416fa4382383f09b3b853b90aa69 Mon Sep 17 00:00:00 2001 From: b-long Date: Mon, 4 May 2026 14:35:29 -0400 Subject: [PATCH 03/10] Restrict .so exports to PyInit__ to prevent runtime symbol interposition --- SUPPORT_MATRIX.md | 1 + _examples/gilstring/gilstring.go | 14 +++++++ _examples/gilstring/test.py | 18 ++++++++ bind/gen.go | 15 +++++++ cmd_build.go | 53 ++++++++++++++++++++---- main_test.go | 71 +++++++++++++++++++++++++++++++- 6 files changed, 162 insertions(+), 10 deletions(-) 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/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..cb58a5e1 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -281,7 +281,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. 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 #370 / #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: 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/main_test.go b/main_test.go index c5f6c29e..c9de380c 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{ @@ -785,6 +784,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 b062959cb5d11bced29f9041538b007184af3615 Mon Sep 17 00:00:00 2001 From: b-long Date: Wed, 6 May 2026 07:22:31 -0400 Subject: [PATCH 04/10] Clear Go TLS slot before each CGo entry to prevent multi-runtime heap corruption --- bind/gen.go | 58 ++++++++++++++++++++++++++++++++++++++++++++---- bind/gen_func.go | 9 +++++--- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/bind/gen.go b/bind/gen.go index cb58a5e1..63de6124 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -87,14 +87,38 @@ 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 ) +// init enforces GOMAXPROCS=1 as a belt-and-suspenders measure: the Python wrapper +// also sets the GOMAXPROCS env var before dlopen, but calling it here guarantees +// the limit even if the extension is loaded without the wrapper (issue #370). +func init() { + runtime.GOMAXPROCS(1) +} + // main doesn't do anything in lib / pkg mode, but is essential for exe mode func main() { %[7]s @@ -259,6 +283,7 @@ 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('_gopy_clear_go_tls', None, []) ` // appended to imports in py wrap preamble as key for adding at end @@ -281,10 +306,35 @@ 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. 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 #370 / #385). +# When multiple gopy extensions coexist in one Python process each carries its own +# independent Go runtime. Three environment variables must be set *before* dlopen +# so the Go runtime reads them during its __attribute__((constructor)): +# +# GOGC=off – disables automatic GC in this extension's runtime (issue #370). +# Root cause: exitsyscall()'s fast path does not call +# prepareForSweep(), so if the GC advances mheap_.sweepgen while +# the cgo goroutine is parked between calls, the cached span has +# a stale sweepgen and the next refill() check panics. Disabling +# GC prevents sweepgen from ever advancing. Memory held by Go +# objects in this extension accumulates until the process exits; +# set GOGC=100 before importing to re-enable GC if your workload +# manages object lifetimes carefully. +# GOMAXPROCS=1 – limits each runtime to one OS-level P, reducing the window in +# which background goroutines from different runtimes overlap. +# asyncpreemptoff=1 – disables SIGURG-based goroutine preemption so the second +# runtime's signal handler cannot fire inside the first's goroutine. +if 'GOGC' not in os.environ: + os.environ['GOGC'] = 'off' +if 'GOMAXPROCS' not in os.environ: + os.environ['GOMAXPROCS'] = '1' +_gopy_godebug = os.environ.get('GODEBUG', '') +if 'asyncpreemptoff' not in _gopy_godebug: + _gopy_godebug = (_gopy_godebug + ',asyncpreemptoff=1').lstrip(',') +os.environ['GODEBUG'] = _gopy_godebug +del _gopy_godebug +# 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 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") From 457462fa008422dfa64776c6e94252c9de5e4a80 Mon Sep 17 00:00:00 2001 From: b-long Date: Wed, 6 May 2026 13:39:29 -0400 Subject: [PATCH 05/10] Remove GOGC=off and GOMAXPROCS mitigations; skip TestCStrings on darwin --- bind/gen.go | 36 +++--------------------------------- go.sum | 4 ++++ main_test.go | 11 ++++++++++- 3 files changed, 17 insertions(+), 34 deletions(-) diff --git a/bind/gen.go b/bind/gen.go index 63de6124..f9952ae7 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -107,18 +107,10 @@ static void _gopy_clear_go_tls(void) { */ import "C" import ( - "runtime" "github.com/go-python/gopy/gopyh" // handler %[6]s ) -// init enforces GOMAXPROCS=1 as a belt-and-suspenders measure: the Python wrapper -// also sets the GOMAXPROCS env var before dlopen, but calling it here guarantees -// the limit even if the extension is loaded without the wrapper (issue #370). -func init() { - runtime.GOMAXPROCS(1) -} - // main doesn't do anything in lib / pkg mode, but is essential for exe mode func main() { %[7]s @@ -307,31 +299,9 @@ 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. Three environment variables must be set *before* dlopen -# so the Go runtime reads them during its __attribute__((constructor)): -# -# GOGC=off – disables automatic GC in this extension's runtime (issue #370). -# Root cause: exitsyscall()'s fast path does not call -# prepareForSweep(), so if the GC advances mheap_.sweepgen while -# the cgo goroutine is parked between calls, the cached span has -# a stale sweepgen and the next refill() check panics. Disabling -# GC prevents sweepgen from ever advancing. Memory held by Go -# objects in this extension accumulates until the process exits; -# set GOGC=100 before importing to re-enable GC if your workload -# manages object lifetimes carefully. -# GOMAXPROCS=1 – limits each runtime to one OS-level P, reducing the window in -# which background goroutines from different runtimes overlap. -# asyncpreemptoff=1 – disables SIGURG-based goroutine preemption so the second -# runtime's signal handler cannot fire inside the first's goroutine. -if 'GOGC' not in os.environ: - os.environ['GOGC'] = 'off' -if 'GOMAXPROCS' not in os.environ: - os.environ['GOMAXPROCS'] = '1' -_gopy_godebug = os.environ.get('GODEBUG', '') -if 'asyncpreemptoff' not in _gopy_godebug: - _gopy_godebug = (_gopy_godebug + ',asyncpreemptoff=1').lstrip(',') -os.environ['GODEBUG'] = _gopy_godebug -del _gopy_godebug +# 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). 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 c9de380c..7cb19a04 100644 --- a/main_test.go +++ b/main_test.go @@ -12,6 +12,7 @@ import ( "os/exec" "path/filepath" "reflect" + "runtime" "sort" "strings" "testing" @@ -767,13 +768,21 @@ OK func TestCStrings(t *testing.T) { // t.Parallel() + if runtime.GOOS == "darwin" { + // On macOS, ru_maxrss is in bytes and the GC behaviour under the + // needm/dropm per-call cycle (required by the multi-extension TLS + // fix for issue #370) means Go allocations are not reclaimed fast + // enough for the leak threshold used by this test. CI runs on + // Linux/Windows where ru_maxrss is in KB, making the threshold + // unreachable, so the test is meaningful only there. + t.Skip("TestCStrings: unreliable on macOS due to maxrss semantics and multi-extension GC trade-off (issue #370)") + } path := "_examples/cstrings" testPkg(t, pkg{ path: path, 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 From aa42e5f8b9bb34798efc97f924b59b7b7d65f3d5 Mon Sep 17 00:00:00 2001 From: b-long Date: Thu, 7 May 2026 19:45:10 -0400 Subject: [PATCH 06/10] Update Skip for TestCStrings (skip amd64 only) --- main_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/main_test.go b/main_test.go index 7cb19a04..9ca67585 100644 --- a/main_test.go +++ b/main_test.go @@ -768,14 +768,14 @@ OK func TestCStrings(t *testing.T) { // t.Parallel() - if runtime.GOOS == "darwin" { - // On macOS, ru_maxrss is in bytes and the GC behaviour under the - // needm/dropm per-call cycle (required by the multi-extension TLS + if runtime.GOOS == "darwin" && runtime.GOARCH == "amd64" { + // On darwin/amd64, ru_maxrss is in bytes and the GC behaviour under + // the needm/dropm per-call cycle (required by the multi-extension TLS // fix for issue #370) means Go allocations are not reclaimed fast // enough for the leak threshold used by this test. CI runs on // Linux/Windows where ru_maxrss is in KB, making the threshold // unreachable, so the test is meaningful only there. - t.Skip("TestCStrings: unreliable on macOS due to maxrss semantics and multi-extension GC trade-off (issue #370)") + t.Skip("TestCStrings: unreliable on darwin/amd64 due to maxrss semantics and multi-extension GC trade-off (issue #370)") } path := "_examples/cstrings" testPkg(t, pkg{ From c1605264031100fa44affdaf6c465a74572632c8 Mon Sep 17 00:00:00 2001 From: b-long Date: Thu, 7 May 2026 20:26:56 -0400 Subject: [PATCH 07/10] Fix off-by-one in cpkg_sprintf: allocate strlen+1 for null terminator malloc(strlen(str)) left no room for the null terminator; sprintf wrote one byte past the end, corrupting the next heap chunk. Previously masked by GOGC=off; now detected by glibc malloc with normal GC enabled. --- _examples/cgo/cgo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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; //} From e65abded3f81c04b5bfb9c43f9867a4fed55f14e Mon Sep 17 00:00:00 2001 From: b-long Date: Thu, 7 May 2026 21:05:51 -0400 Subject: [PATCH 08/10] Add macos-15-intel to CI matrix --- .github/workflows/ci.yml | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e232aa0c..25cba8c4 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] #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: matrix.platform == 'macos-15-intel' + 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: matrix.platform == 'macos-15-intel' + run: | + make + - name: Test macOS + if: matrix.platform == 'macos-15-intel' + run: | + make test + - name: Build-Windows if: matrix.platform == 'windows-latest' run: | From bd6703d8e97c7e9ab7adffe5c7d770bed466c60e Mon Sep 17 00:00:00 2001 From: b-long Date: Thu, 7 May 2026 21:07:47 -0400 Subject: [PATCH 09/10] Add macos-15 (ARM) to CI matrix; generalize macOS step conditions --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 25cba8c4..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, macos-15-intel] + platform: [ubuntu-latest, windows-latest, macos-15-intel, macos-15] #platform: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.platform }} steps: @@ -52,7 +52,7 @@ jobs: go install golang.org/x/tools/cmd/goimports@latest - name: Install macOS packages - if: matrix.platform == 'macos-15-intel' + if: startsWith(matrix.platform, 'macos-') run: | python3 -m pip install -U pybindgen go install golang.org/x/tools/cmd/goimports@latest @@ -76,11 +76,11 @@ jobs: make test - name: Build-macOS - if: matrix.platform == 'macos-15-intel' + if: startsWith(matrix.platform, 'macos-') run: | make - name: Test macOS - if: matrix.platform == 'macos-15-intel' + if: startsWith(matrix.platform, 'macos-') run: | make test From d4f84a2e25eae6673ca7625075b9a1b6bca37688 Mon Sep 17 00:00:00 2001 From: b-long Date: Thu, 7 May 2026 22:04:20 -0400 Subject: [PATCH 10/10] Sync Go GC with Python GC via gc.callbacks to fix TestCStrings on macOS Python's gc.collect() releases Python wrappers and calls DecRef(), removing Go objects from the handle map. However, Go's own GC does not run at that point, so Go-heap memory (structs, slices) stays resident until Go's allocator threshold is reached. On macOS, ru_maxrss is in bytes instead of KB, making the leak threshold meaningful (~82 MB), so the second test pass can see RSS growth before Go GC has had a chance to reclaim the first pass's allocations -- causing a false-positive leak report. Fix in bind/gen.go: - Add RunGC() (calls runtime.GC()) to the generated Go extension. - Register a Python gc.callbacks handler in the generated Python wrapper that calls _{pkg}.RunGC() whenever Python GC stops. This is transparent to users: Go GC fires automatically after every Python GC cycle, keeping the two runtimes in sync without any user-visible API change. Remove the darwin skip from main_test.go: now that Go GC is synchronised with Python GC, TestCStrings is reliable on all platforms including both macOS/Intel and macOS/ARM. --- bind/gen.go | 24 ++++++++++++++++++++++++ main_test.go | 10 ---------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/bind/gen.go b/bind/gen.go index f9952ae7..34a5c375 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -107,6 +107,7 @@ static void _gopy_clear_go_tls(void) { */ import "C" import ( + "runtime" "github.com/go-python/gopy/gopyh" // handler %[6]s ) @@ -148,6 +149,15 @@ func NumHandles() int { return gopyh.NumHandles() } +// RunGC runs the Go garbage collector. gopy registers this as a Python +// gc.callbacks handler so it fires automatically after each Python GC cycle, +// keeping Go-heap objects freed via DecRef actually collected without any +// user intervention. +//export RunGC +func RunGC() { + runtime.GC() +} + // boolGoToPy converts a Go bool to python-compatible C.char func boolGoToPy(b bool) C.char { if b { @@ -275,6 +285,7 @@ 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('RunGC', None, []) mod.add_function('_gopy_clear_go_tls', None, []) ` @@ -318,6 +329,19 @@ else: 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.RunGC() + _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/main_test.go b/main_test.go index 9ca67585..1127698c 100644 --- a/main_test.go +++ b/main_test.go @@ -12,7 +12,6 @@ import ( "os/exec" "path/filepath" "reflect" - "runtime" "sort" "strings" "testing" @@ -768,15 +767,6 @@ OK func TestCStrings(t *testing.T) { // t.Parallel() - if runtime.GOOS == "darwin" && runtime.GOARCH == "amd64" { - // On darwin/amd64, ru_maxrss is in bytes and the GC behaviour under - // the needm/dropm per-call cycle (required by the multi-extension TLS - // fix for issue #370) means Go allocations are not reclaimed fast - // enough for the leak threshold used by this test. CI runs on - // Linux/Windows where ru_maxrss is in KB, making the threshold - // unreachable, so the test is meaningful only there. - t.Skip("TestCStrings: unreliable on darwin/amd64 due to maxrss semantics and multi-extension GC trade-off (issue #370)") - } path := "_examples/cstrings" testPkg(t, pkg{ path: path,