Skip to content

sftp.readdir (and other high-level SFTP methods) silently drop callback on connection loss #1490

@jeffrson

Description

@jeffrson

When the remote server crashes or the TCP connection drops abruptly, high-level SFTP methods such as sftp.readdir(path, cb) never invoke their
callback — neither with a result nor with an error. The calling code stalls silently with no way to detect the failure.

Reproducer (https://github.com/jeffrson/ssh2-missing-cb)

  sftp.readdir('/some/path', (err, list) => {
    // never called when server dies mid-request
    console.log(err, list);
  });

  // simulate server crash while readdir is in flight
  setTimeout(() => process.kill(serverPid), 50);

Root cause

cleanupRequests() in SFTP.js is designed to drain all pending _requests with a 'No response from server' error when the channel closes. It is called
correctly from push(null) on connection loss.

The problem is that high-level methods like readdir (string form, lines 886–910) call further SFTP operations inside their error handlers:

  this.readdir(handle, opts, (err, list) => {
    if (err && !eof)
      return this.close(handle, () => cb(err));  // ← adds a new _requests entry
    ...
  });

The sequence on connection drop:

  1. cleanupRequests fires → calls the pending READDIR callback with err
  2. That callback synchronously calls this.close(handle, () => cb(err))
  3. close() registers a new entry in _requests
  4. cleanupRequests has already returned; channel.readable is now false → push(null) will never fire again
  5. The new close request hangs forever → user callback is never called

This affects every high-level SFTP method that calls a further SFTP operation in its error or EOF handler. readdir with a string path is the most
common case, but the pattern recurs elsewhere.

Fix

cleanupRequests must loop until _requests is stably empty, so that any requests added synchronously by cleanup callbacks are also flushed:

  function cleanupRequests(sftp) {
    const err = new Error('No response from server');
    while (true) {
      const keys = Object.keys(sftp._requests);
      if (keys.length === 0)
        return;
      const reqs = sftp._requests;
      sftp._requests = {};
      for (let i = 0; i < keys.length; ++i) {
        const req = reqs[keys[i]];
        if (typeof req.cb === 'function')
          req.cb(err);
      }
    }
  }

Each iteration processes requests added by the previous round's callbacks. The loop terminates because callback chains are finite in depth (e.g. readdir → close → cb → done, depth 2).

Edit: I'm sorry I missed my older report (it's rather old so that's why...): #1175 - however, this time the report contains deeper analysis and has a PR.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions