Skip to content

fix(sftp): flush synchronously added requests during connection cleanup#1491

Open
jeffrson wants to merge 1 commit intomscdex:masterfrom
jeffrson:readdir
Open

fix(sftp): flush synchronously added requests during connection cleanup#1491
jeffrson wants to merge 1 commit intomscdex:masterfrom
jeffrson:readdir

Conversation

@jeffrson
Copy link
Copy Markdown

@jeffrson jeffrson commented Apr 4, 2026

Fixes #1490

Problem

cleanupRequests() runs once when the channel closes and calls all pending _requests callbacks with an error. However, some callbacks (notably the high-level readdir error path) synchronously register new entries in _requests via this.close(handle, cb). These new entries are never flushed — the channel is already non-readable, so push(null) and therefore cleanupRequests will not fire again. The user-supplied callback is silently dropped.

Fix

Replace the single-pass cleanup with a loop that keeps flushing until _requests is stably empty:

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

The loop terminates because callback chains are finite (typical depth: readdir → close → user callback → done). The err object is created once outside the loop to avoid redundant allocations.

The "reproducer" at https://github.com/jeffrson/ssh2-missing-cb finally executes the callback in client when this patch is used.

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.

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

1 participant