Skip to content

fix(client): dispose credentials subscription on every close() path - #3391

Merged
nkaradzhov merged 1 commit into
redis:masterfrom
lazerg:fix/close-dispose-credentials-3390
Aug 5, 2026
Merged

fix(client): dispose credentials subscription on every close() path#3391
nkaradzhov merged 1 commit into
redis:masterfrom
lazerg:fix/close-dispose-credentials-3390

Conversation

@lazerg

@lazerg lazerg commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #3390.

close() only disposed the streaming credentials subscription on the path where the command queue still had pending commands. On a graceful shutdown of an idle client the queue is already empty, so close() hit the early return resolve() and left the subscription alive. With @redis/entraid that keeps the token-refresh timer armed and the process never exits.

The dispose call now runs before the empty-queue early return, so both paths release the subscription, matching what destroy() already does.

The new test connects a client with a streaming credentials provider to a mock TCP server, closes it with an empty queue, and asserts the subscription was disposed.


Checklist

  • Does npm test pass with this change (including linting)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?

Note

Low Risk
Small lifecycle fix aligned with destroy()/quit(); behavior change only affects graceful close with streaming auth when the queue is empty.

Overview
close() now disposes the streaming credentialsSubscription at the start of shutdown, before the command-queue empty check. Previously that cleanup only ran on the branch that waited for pending replies, so an idle client could exit close() without calling dispose()—leaving token-refresh timers alive (e.g. with @redis/entraid) and preventing process exit.

A regression test connects with a streaming credentials provider, **close()**s with an empty queue, and asserts dispose ran. countRespCommands was hoisted to module scope for shared mock-server use.

Reviewed by Cursor Bugbot for commit 1af6ea8. Bugbot is set up for automated code reviews on this repo. Configure here.

@nkaradzhov nkaradzhov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@lazerg thanks, this looks good to go!

@nkaradzhov
nkaradzhov merged commit 0c03255 into redis:master Aug 5, 2026
15 checks passed
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.

close() skips disposing the credentials subscription when the command queue is empty, leaking the entraid token-refresh timer

2 participants