Skip to content

PHP: fix memory leaks and resource management issues#174

Open
prateek-kumar-improving wants to merge 2 commits intomainfrom
php/fix-memory-issues
Open

PHP: fix memory leaks and resource management issues#174
prateek-kumar-improving wants to merge 2 commits intomainfrom
php/fix-memory-issues

Conversation

@prateek-kumar-improving
Copy link
Copy Markdown
Collaborator

@prateek-kumar-improving prateek-kumar-improving commented Mar 26, 2026

Summary

Fixes multiple memory leaks, a potential crash in debug builds, and a resource management gap.

Issue link

This Pull Request is linked to issue (URL): #175

Features / Behaviour Changes

  • ValkeyGlide::close() and ValkeyGlideCluster::close() now actually release the underlying Rust client connection instead of being no-ops. Previously, connections were only released
    on garbage collection.

Implementation

# Fix File(s)
1 Check EG(exception) after _initialize_open_telemetry() — was returning SUCCESS with a pending exception, leaking config and causing undefined behavior in FFI valkey_glide.c
2 Replace bare return NULL with goto cleanup_route in execute_command_with_route() validation checks — route_bytes and route.key were leaked command_response.c
3 Guard efree(offset_str) / efree(count_str) with NULL checks in sort commands — crashes in PHP debug builds when no LIMIT option valkey_glide_str_commands.c
4 Free buffered_commands in free_valkey_glide_object() — batch state leaked if object destroyed mid-batch valkey_glide.c
5 Implement close() to call close_glide_client() — was a no-op, connections only released on GC valkey_glide.c, valkey_glide_cluster.c

Testing

All integration tests will run.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one issue.
  • Commit message has a detailed description of what changed and why.
  • Tests are added or updated.
  • CHANGELOG.md and documentation files are updated.
  • Destination branch is correct - main or release
  • Create merge commit if merging release branch into main, squash otherwise.

Signed-off-by: Prateek Kumar <prateek.kumar@improving.com>
@prateek-kumar-improving prateek-kumar-improving changed the title php: fix memory leaks and resource management issues PHP: fix memory leaks and resource management issues Mar 26, 2026
void free_valkey_glide_object(zend_object* object) {
valkey_glide_object* valkey_glide = VALKEY_GLIDE_PHP_GET_OBJECT(valkey_glide_object, object);

/* Free buffered batch commands if object is destroyed mid-batch */
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should the buffered commands also be cleaned up when close is called? This could be moved to a helper function and called here and in close.

This also applies to valkey_glide_cluster.c file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants