Fix memory leaks & crashes when loading multiple Go extensions in one Python process#393
Open
b-long wants to merge 5 commits into
Open
Fix memory leaks & crashes when loading multiple Go extensions in one Python process#393b-long wants to merge 5 commits into
b-long wants to merge 5 commits into
Conversation
…ne 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.
This was referenced May 13, 2026
rcoreilly
approved these changes
May 13, 2026
Member
rcoreilly
left a comment
rcoreilly
left a comment
There was a problem hiding this comment.
Just from a quick read-through of the code, everything you have in here corresponds to what you described in the PR comment. Amazing work by multiple people to figure out these really arcane and random issues at the interface between these two languages -- I certainly never would have figured any of that out! And this approval by no means indicates that I understand the details of the loader and those needm assembly registers (wow!) but the tests don't lie presumably :)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends-on: #392
Fixes: #385
Fixes: #370
I took Scusemua's two commits in #361 as a base and built a broader set of fixes on top, covering memory leaks, GC coordination, and shared library conflicts.
This PR wouldn't exist without the work Scusemua put in first. His two commits from July 2024 fixed a subtle but nasty bug: Go code was calling a Python function without holding the GIL.
That's a precise fix that prevents several hard-to-reproduce crashes, and everything else here builds on top of it.
I added four fixes in the same area:
C string memory leaks. Generated code allocated C strings when reading string fields from Go structs, slices, and maps, but never freed them. Switched those paths to a helper that frees after Python copies the value.
Unsafe GC call. The generated wrapper was calling Go's GC from a thread that crosses the C/Go boundary, which could panic on multi-core machines. Rerouted it through a background goroutine via a channel.
GC out of sync. Without coordination, Go heap objects piled up between Python GC cycles. Fixed by registering a gc.callbacks handler that triggers a Go collection after each Python one.
Symbol conflicts between co-loaded extensions. Loading two gopy-built shared libraries with RTLD_GLOBAL caused one library's Go internals to clobber the other's, corrupting heap state. Fixed by loading without that flag so each library's runtime stays isolated.
I added a regression test for two extensions in a single Python process, stealing from the discussion in issue 370.
I also expanded the CI matrix to cover more Go versions and macOS configs.