Skip to content

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

Description

@cuzzlor

Environment

  • Node.js Version: 24.x
  • Redis Server Version: Azure Managed Redis (Enterprise cluster policy)
  • Node Redis Version: 5.12.1 (bug also present on current master, permalinks below)
  • Platform: Linux / macOS

Description

When a client is constructed with a credentialsProvider from @redis/entraid, client.close() only disposes the credentials subscription on one of its two exit paths. The #credentialsSubscription?.dispose() call sits after the empty-queue early return:

close() {
return new Promise<void>(resolve => {
clearTimeout(this._self.#pingTimer);
this._self.#socket.close();
this._self.#clientSideCache?.onClose();
if (this._self.#queue.isEmpty()) {
this._self.#unregisterFromMetrics();
this._self.#socket.destroySocket();
return resolve();
}
const maybeClose = () => {
if (!this._self.#queue.isEmpty()) return;
this._self.#socket.off('data', maybeClose);
this._self.#unregisterFromMetrics();
this._self.#socket.destroySocket();
resolve();
};
this._self.#socket.on('data', maybeClose);
this._self.#credentialsSubscription?.dispose();
this._self.#credentialsSubscription = null;
});
}

  • Queue not empty: the code falls through to #credentialsSubscription?.dispose() — subscription disposed correctly.
  • Queue empty (the normal case for a graceful shutdown — all commands already settled): close() returns at the early return resolve() and the credentials subscription is never disposed.

The undisposed subscription leaves the entraid TokenManager refresh timer armed, which keeps the event loop alive — the process hangs after close() resolves instead of exiting.

destroy() disposes the subscription unconditionally, which is the behaviour close() presumably intends on both paths:

destroy() {
clearTimeout(this._self.#pingTimer);
this._self.#queue.flushAll(new DisconnectsClientError());
this._self.#socket.destroy();
this._self.#clientSideCache?.onClose();
this._self.#unregisterFromMetrics();
this._self.#credentialsSubscription?.dispose();
this._self.#credentialsSubscription = null;
}

Reproduction

import { createClient } from 'redis';
import { EntraIdCredentialsProviderFactory, REDIS_SCOPE_DEFAULT } from '@redis/entraid';
import { DefaultAzureCredential } from '@azure/identity';

const client = createClient({
  url: 'rediss://<amr-instance>.redis.azure.net:10000',
  RESP: 3,
  credentialsProvider: EntraIdCredentialsProviderFactory.createForDefaultAzureCredential({
    credential: new DefaultAzureCredential(),
    scopes: REDIS_SCOPE_DEFAULT,
    tokenManagerConfig: { expirationRefreshRatio: 0.8 },
  }),
});

await client.connect();
await client.ping();      // all commands settle — queue is now empty
await client.close();     // resolves, but the token-refresh timer stays armed
// process does not exit

Swapping close() for destroy() (or calling destroy() after close()) lets the process exit, confirming the credentials subscription is what holds the loop.

Expected behaviour

close() disposes the credentials subscription on every exit path, as destroy() does — a graceful close of an idle client should release everything the client owns.

Workaround

Quiesce then destroy: await client.close() (drains pending replies) followed by client.destroy() (releases the credentials subscription). We run this in production against Azure Managed Redis with Entra ID auth; it behaves correctly.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions