redis / node-redis

Redis Node.js client
https://redis.js.org/
MIT License
16.9k stars 1.89k forks source link

Aborted blocking commands leave dangling sockets #1889

Open qubyte opened 2 years ago

qubyte commented 2 years ago

Environment:

Abort signals appear to result in sockets being left open. Consider this code:

"use strict";

const redis = require("redis");
const client = redis.createClient();

client.connect().then(() => {
  const abortController = new AbortController();
  const options = redis.commandOptions({ signal: abortController.signal, isolated: true });

  client.blPop(options, 'test-list', 0).catch(error => {
    console.log('error is AbortError', error instanceof redis.AbortError);
  });

  setTimeout(async () => {
    abortController.abort();
    await client.quit();
    console.log("client quit")
  }, 100);
});

The abort signal triggers the expected console log about the AbortError. However, the node process will not exit. The second log also happens, so we know that the client thinks it has cleared itself up. client.disconnect() yields the same result.

More observations:

Augmenting this code with a utility like wtfnode shows an open socket to redis which should be closed (on various ports on the left):

[WTF Node?] open handles:
- File descriptors: (note: stdio always exists)
  - fd 1 (tty) (stdio)
  - fd 2 (tty) (stdio)
- Sockets:
  - (?:?) -> localhost:? (destroyed)
  - 127.0.0.1:50820 -> 127.0.0.1:6379
- Timers:
  - (100 ~ 100 ms) (anonymous) @ /Volumes/code/.../test_bug.js:15

My guess (which is probably way off) is that the destroyed socket is the client directly created and quit in the code, but the indirectly created isolated client is not being cleaned up.

There's not much information in the docs about abort signals. It may be I've stumbled upon some API which isn't quite ready yet. My apologies if so!

leibale commented 2 years ago

The thing is that a command can be aborted only before it was written on the socket, after that, even do the promise is still pending, you cannot abort the command (We can reject the promise, but the server will still process that command...)

hesxenon commented 2 years ago

it's not about resolving or rejecting the promise though, it's about closing all sockets when client.quit() is called which is not the case.

I assume the abort signal is in this example only to demonstrate that node does indeed not quit because of the open socket and not because of the blocking command.

hesxenon commented 2 years ago

@qubyte I don't know if you've solved this by now, I was just able to work around this by using client.executeIsolated and closing the isolated client explicitly (along with the "original" client)