Skip to content

fix(bindings/go): release owned c strings via c api#7403

Merged
tisonkun merged 3 commits into
apache:mainfrom
jihuayu:codex/fix-go-string-ownership
May 12, 2026
Merged

fix(bindings/go): release owned c strings via c api#7403
tisonkun merged 3 commits into
apache:mainfrom
jihuayu:codex/fix-go-string-ownership

Conversation

@jihuayu
Copy link
Copy Markdown
Member

@jihuayu jihuayu commented Apr 17, 2026

Rationale for this change

The Go binding was not releasing heap-allocated strings returned by the C API after converting them into Go strings.

This could lead to memory leaks in repeated metadata queries such as Info() and during large list operations. In addition, the C binding did not provide a dedicated API for releasing owned strings, which made the ownership contract harder to follow for managed-language bindings built on top of the C API.

What changes are included in this PR?

This PR introduces a dedicated C API to release owned strings returned by the C binding and updates the Go binding to use it when handling string results.

It also updates the related C binding documentation and tests to reflect the new ownership contract, and adds Go regression tests to cover the affected paths.

Are there any user-facing changes?

No breaking user-facing API changes.

The main visible change is that the C binding now provides a dedicated string release API for bindings that consume owned strings from the C layer.

AI Usage Statement

This PR was developed with the help of OpenAI Codex.

@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. releases-note/fix The PR fixes a bug or has a title that begins with "fix" labels Apr 17, 2026
@jihuayu
Copy link
Copy Markdown
Member Author

jihuayu commented Apr 17, 2026

If you feel this would be better handled as two separate PRs, please let me know.

@jihuayu
Copy link
Copy Markdown
Member Author

jihuayu commented May 12, 2026

Call my dear extraordinary review agent. @Xuanwo Can you have a look?

cc @tisonkun. Thanks for teaching me how to ping people.

Copy link
Copy Markdown
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution @jihuayu! LGTM.

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label May 12, 2026
@tisonkun tisonkun merged commit cdf5762 into apache:main May 12, 2026
91 of 100 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer releases-note/fix The PR fixes a bug or has a title that begins with "fix" size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants