From 43e1f403115fb9846013aa3f6139b78824762e39 Mon Sep 17 00:00:00 2001 From: b-long Date: Thu, 30 Apr 2026 23:37:29 +0000 Subject: [PATCH 1/3] =?UTF-8?q?add=20regression=20tests=20for=20GIL=20orde?= =?UTF-8?q?ring=20bug=20(issue=20#370)=20=E2=80=94=20tests=20only,=20no=20?= =?UTF-8?q?fix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds TestGilString and expands TestBindCgoPackage/TestBindSimple to exercise the go2py string conversion paths that panic on master without the fix. Intended to run against master to prove the bug is reproducible in CI. --- _examples/cgo/cgo.go | 2 +- _examples/cgo/test.py | 8 +++++++ _examples/gilstring/gilstring.go | 38 ++++++++++++++++++++++++++++++++ _examples/gilstring/test.py | 37 +++++++++++++++++++++++++++++++ main_test.go | 16 ++++++++++++-- 5 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 _examples/gilstring/gilstring.go create mode 100644 _examples/gilstring/test.py 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..90fb0db5 --- /dev/null +++ b/_examples/gilstring/gilstring.go @@ -0,0 +1,38 @@ +// 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 + +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 + 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..1b0b56b2 --- /dev/null +++ b/_examples/gilstring/test.py @@ -0,0 +1,37 @@ +# 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). +# 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"] + +print("OK") diff --git a/main_test.go b/main_test.go index c5f6c29e..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() @@ -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{ @@ -557,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 `), }) @@ -785,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 5b00113918c4e20063abb30a645616840496a716 Mon Sep 17 00:00:00 2001 From: b-long Date: Thu, 30 Apr 2026 23:46:46 +0000 Subject: [PATCH 2/3] update SUPPORT_MATRIX.md for gilstring example --- SUPPORT_MATRIX.md | 1 + 1 file changed, 1 insertion(+) 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 From 415c437f1a69709252d8fb182ceb4466dcfa1589 Mon Sep 17 00:00:00 2001 From: b-long Date: Fri, 1 May 2026 00:01:00 +0000 Subject: [PATCH 3/3] ci: add macos-13 (x86_64) to test matrix to reproduce issue #370 --- .github/workflows/ci.yml | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e232aa0c..f4d5a531 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-13] #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-13' + 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,15 @@ jobs: if: matrix.platform == 'windows-latest' run: | go test -v ./... + + - name: Build-macOS + if: matrix.platform == 'macos-13' + run: | + make + - name: Test macOS + if: matrix.platform == 'macos-13' + run: | + make test - name: Upload-Coverage if: matrix.platform == 'ubuntu-latest' uses: codecov/codecov-action@v4