Skip to content

fix(cgo): resolve CStrings memory leaks#249

Merged
github-actions[bot] merged 1 commit into
llm-d:mainfrom
sagearc:fix-cgo-string-memleak
Jan 7, 2026
Merged

fix(cgo): resolve CStrings memory leaks#249
github-actions[bot] merged 1 commit into
llm-d:mainfrom
sagearc:fix-cgo-string-memleak

Conversation

@sagearc
Copy link
Copy Markdown
Collaborator

@sagearc sagearc commented Jan 6, 2026

closes #248

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Copilot AI review requested due to automatic review settings January 6, 2026 12:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes memory leaks in CGO code by properly freeing CStrings allocated with C.CString(). The issue was that two functions were creating CStrings to pass JSON data to Python functions but never releasing the allocated memory.

  • Added proper memory management for CString allocations in two functions
  • Both functions now store the CString in a variable and use defer C.free() to ensure cleanup

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@hyeongyun0916
Copy link
Copy Markdown
Collaborator

This looks great. I think we should apply the same logic to this as well.

@vMaroon
Copy link
Copy Markdown
Member

vMaroon commented Jan 7, 2026

/lgtm
/approve

@github-actions github-actions Bot added the lgtm Looks good to me, indicates that a PR is ready to be merged. label Jan 7, 2026
@github-actions github-actions Bot merged commit b5dd010 into llm-d:main Jan 7, 2026
11 checks passed
@sagearc sagearc deleted the fix-cgo-string-memleak branch January 7, 2026 18:11
guygir pushed a commit to guygir/llm-d-kv-cache-manager that referenced this pull request Apr 20, 2026
* build: change epp-config default yaml
* build: change imagePullPolicy to IfNotPresent for simulation

Signed-off-by: Kfir Toledo <kfir.toledo@ibm.com>

---------

Signed-off-by: Kfir Toledo <kfir.toledo@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Looks good to me, indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CGO memory leaks

4 participants