Skip to content

Commit 0223539

Browse files
committed
test: rewrite TestGilString to match exact issue #370 reproduction
Rebuilds the gilstring example as a minimal two-extension test: gilstring (Hello) and simple (Add) are each built into the same workdir and imported in the same Python process, interleaved over 5000 iterations — matching the exact script from the issue report. Replaces the single-extension approach that could not trigger the cross-extension GIL/cgo crash.
1 parent f93fc93 commit 0223539

3 files changed

Lines changed: 74 additions & 65 deletions

File tree

_examples/gilstring/gilstring.go

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,36 +3,12 @@
33
// license that can be found in the LICENSE file.
44

55
// Package gilstring is a regression test for the GIL ordering bug (issue #370).
6-
// It exercises go2py string conversion through functions, struct fields,
7-
// slice elements, and map values — all of which previously ran C.CString
8-
// without holding the GIL, causing crashes under repeated calls.
6+
// It mirrors the exact reproduction from the issue report: a string-returning
7+
// function called alongside an integer function from a second extension in the
8+
// same Python process, which triggers crashes under repeated calls.
99
package gilstring
1010

1111
import "fmt"
1212

13-
// Add returns the sum of its arguments, mirroring simple.Add from the issue
14-
// report reproduction script.
15-
func Add(i, j int) int { return i + j }
16-
17-
// Hello returns a greeting string, mirroring hi.Hello from the issue report
18-
// reproduction script. Returning a non-trivial string stresses go2py
19-
// string conversion (C.CString) on every call.
13+
// Hello returns a greeting string, mirroring hi.Hello from the issue report.
2014
func Hello(s string) string { return fmt.Sprintf("Hello, %s!", s) }
21-
22-
// Item is a struct with a string field to exercise struct member string getters.
23-
type Item struct {
24-
Label string
25-
Count int
26-
}
27-
28-
// MakeItem returns an Item with the given label.
29-
func MakeItem(s string) Item { return Item{Label: s, Count: len(s)} }
30-
31-
// GetLabel returns the Label field of an Item.
32-
func GetLabel(i Item) string { return i.Label }
33-
34-
// StringSlice returns a slice of strings.
35-
func StringSlice() []string { return []string{"alpha", "beta", "gamma"} }
36-
37-
// StringMap returns a map with string values.
38-
func StringMap() map[string]string { return map[string]string{"key": "value"} }

_examples/gilstring/test.py

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,33 +5,14 @@
55
## py2/py3 compat
66
from __future__ import print_function
77

8-
import gilstring
9-
10-
# Regression test for GIL ordering bug (issue #370 / PR #386).
11-
# Mirrors the exact reproduction script from the issue report:
12-
# for _ in range(5000):
13-
# Add(2, 2)
14-
# Hello('hi')
15-
# Integer arithmetic (Add) interleaved with string-returning calls (Hello)
16-
# stresses the go2py C.CString path that previously ran without the GIL held.
17-
N = 5000
18-
19-
for _ in range(N):
20-
assert gilstring.Add(2, 2) == 4
21-
assert gilstring.Hello("hi") == "Hello, hi!"
22-
23-
# Struct string fields, slice elements, and map values exercise additional
24-
# go2py string conversion paths from the same bug.
25-
for _ in range(N):
26-
item = gilstring.MakeItem("hello")
27-
assert item.Label == "hello", item.Label
28-
29-
for _ in range(N):
30-
s = gilstring.StringSlice()
31-
assert s[0] == "alpha", s[0]
32-
33-
for _ in range(N):
34-
m = gilstring.StringMap()
35-
assert m["key"] == "value", m["key"]
8+
# Regression test for GIL ordering bug (issue #370).
9+
# Exact reproduction from the issue report: two separately-built gopy
10+
# extensions loaded in the same process, with calls interleaved in a loop.
11+
from gilstring.gilstring import Hello
12+
from simple.simple import Add
13+
14+
for _ in range(5000):
15+
Add(2, 2)
16+
Hello('hi')
3617

3718
print("OK")

main_test.go

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -785,16 +785,68 @@ OK
785785
})
786786
}
787787

788+
// TestGilString is a regression test for the GIL ordering bug (issue #370).
789+
// It replicates the exact reproduction from the issue report: two separately-built
790+
// gopy extensions (gilstring and simple) loaded in the same Python process, with
791+
// calls interleaved in a loop of 5000 iterations.
788792
func TestGilString(t *testing.T) {
789-
// t.Parallel()
790-
path := "_examples/gilstring"
791-
testPkg(t, pkg{
792-
path: path,
793-
lang: features[path],
794-
cmd: "build",
795-
extras: nil,
796-
want: []byte("OK\n"),
797-
})
793+
backends := []string{"py3"}
794+
for _, be := range backends {
795+
vm, ok := testBackends[be]
796+
if !ok || vm == "" {
797+
t.Logf("Skipped testing backend %s for TestGilString\n", be)
798+
continue
799+
}
800+
t.Run(be, func(t *testing.T) {
801+
cwd, _ := os.Getwd()
802+
803+
workdir, err := os.MkdirTemp("", "gopy-")
804+
if err != nil {
805+
t.Fatalf("could not create workdir: %v", err)
806+
}
807+
defer os.RemoveAll(workdir)
808+
defer bind.ResetPackages()
809+
810+
// Build gilstring into workdir.
811+
writeGoMod(t, cwd, workdir)
812+
if err := run([]string{"build", "-vm=" + vm, "-output=" + workdir, "./_examples/gilstring"}); err != nil {
813+
t.Fatalf("error building gilstring: %v", err)
814+
}
815+
bind.ResetPackages()
816+
817+
// Build simple into workdir alongside gilstring.
818+
writeGoMod(t, cwd, workdir)
819+
if err := run([]string{"build", "-vm=" + vm, "-output=" + workdir, "./_examples/simple"}); err != nil {
820+
t.Fatalf("error building simple: %v", err)
821+
}
822+
823+
// Copy test.py into workdir.
824+
tstSrc := filepath.Join(cwd, "_examples/gilstring/test.py")
825+
tstDst := filepath.Join(workdir, "test.py")
826+
if err := copyCmd(tstSrc, tstDst); err != nil {
827+
t.Fatalf("error copying test.py: %v", err)
828+
}
829+
830+
env := make([]string, len(testEnvironment))
831+
copy(env, testEnvironment)
832+
env = append(env, fmt.Sprintf("PYTHONPATH=%s", workdir))
833+
834+
cmd := exec.Command(vm, "./test.py")
835+
cmd.Env = env
836+
cmd.Dir = workdir
837+
cmd.Stdin = os.Stdin
838+
buf, err := cmd.CombinedOutput()
839+
if err != nil {
840+
t.Fatalf("error running python module: err=%v\n%s", err, string(buf))
841+
}
842+
843+
got := strings.Replace(string(buf), "\r\n", "\n", -1)
844+
want := "OK\n"
845+
if got != want {
846+
t.Fatalf("got:\n%s\nwant:\n%s", got, want)
847+
}
848+
})
849+
}
798850
}
799851

800852
func TestPackagePrefix(t *testing.T) {

0 commit comments

Comments
 (0)