diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e232aa0c..3a5c585f 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-intel-xlarge] #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' || matrix.platform == 'macos-15-intel-xlarge' + 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,17 @@ jobs: if: matrix.platform == 'windows-latest' run: | go test -v ./... + + - name: Build-macOS + if: matrix.platform == 'macos-15-intel' || matrix.platform == 'macos-15-intel-xlarge' + run: | + make + + - name: Test macOS + if: matrix.platform == 'macos-15-intel' || matrix.platform == 'macos-15-intel-xlarge' + run: | + make test + - name: Upload-Coverage if: matrix.platform == 'ubuntu-latest' uses: codecov/codecov-action@v4 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/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..df160282 --- /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 GIL ordering bug (issue #370). +// It mirrors the exact reproduction from the issue report: a string-returning +// function called alongside an integer function from a second extension in the +// same Python process, which triggers crashes under repeated calls. +package gilstring + +import "fmt" + +// Hello returns a greeting string, mirroring hi.Hello from the issue report. +func Hello(s string) string { return fmt.Sprintf("Hello, %s!", s) } diff --git a/_examples/gilstring/test.py b/_examples/gilstring/test.py new file mode 100644 index 00000000..e4c14818 --- /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 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 c5f6c29e..eeb01d85 100644 --- a/main_test.go +++ b/main_test.go @@ -12,6 +12,7 @@ import ( "os/exec" "path/filepath" "reflect" + "runtime" "sort" "strings" "testing" @@ -49,6 +50,7 @@ var ( "_examples/cstrings": []string{"py3"}, "_examples/pkgconflict": []string{"py3"}, "_examples/variadic": []string{"py3"}, + "_examples/gilstring": []string{"py3"}, } testEnvironment = os.Environ() @@ -316,7 +318,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 +547,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{ @@ -557,6 +557,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 `), }) @@ -767,6 +768,9 @@ OK } func TestCStrings(t *testing.T) { + if runtime.GOOS == "darwin" { + t.Skip("skipping cstrings test on darwin since it leaks all but String") + } // t.Parallel() path := "_examples/cstrings" testPkg(t, pkg{ @@ -785,6 +789,82 @@ 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) { + 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) { // t.Parallel() path := "_examples/package/mypkg"