Jest detects open handles when importing ssh2

[cristianrgreco]

Hello!

When I run a test which imports ssh2 with jest --detectOpenHandles, I get warnings:

Jest has detected the following 4 open handles potentially keeping Jest from exiting:

  ●  SIGNREQUEST

    > 1 | import Dockerode, { DockerOptions, NetworkInspectInfo } from "dockerode";
        | ^
      2 | import path from "path";
      3 | import { log } from "../logger";
      4 | import { Host } from "./types";

      at node_modules/ssh2/lib/protocol/constants.js:22:20
      at Object.<anonymous> (node_modules/ssh2/lib/protocol/constants.js:29:3)
      at Object.<anonymous> (node_modules/ssh2/lib/protocol/keyParser.js:24:46)
      at Object.<anonymous> (node_modules/ssh2/lib/agent.js:9:35)
      at Object.<anonymous> (node_modules/ssh2/lib/index.js:10:5)
      at Object.<anonymous> (node_modules/docker-modem/lib/ssh.js:1:96)
      at Object.<anonymous> (node_modules/docker-modem/lib/modem.js:6:9)
      at Object.<anonymous> (node_modules/dockerode/lib/docker.js:2:11)
      at Object.<anonymous> (src/docker/docker-client.ts:1:1)
      at Object.<anonymous> (src/docker/functions/demux-stream.ts:3:1)
      at Object.<anonymous> (src/docker/functions/container/container-logs.ts:4:1)
      at Object.<anonymous> (src/wait-strategy.ts:9:1)
      at Object.<anonymous> (src/wait.ts:1:1)
      at Object.<anonymous> (src/reaper.ts:9:1)
      at Object.<anonymous> (src/generic-container/generic-container.ts:7:1)
      at Object.<anonymous> (src/generic-container/generic-container.test.ts:4:1)
      at TestScheduler.scheduleTests (node_modules/@jest/core/build/TestScheduler.js:333:13)
      at runJest (node_modules/@jest/core/build/runJest.js:404:19)
      at _run10000 (node_modules/@jest/core/build/cli/index.js:320:7)
      at runCLI (node_modules/@jest/core/build/cli/index.js:173:3)

The code causing the issue is this, specifically crypto.sign(null, data, key);:

const eddsaSupported = (() => {
  if (typeof crypto.sign === 'function'
      && typeof crypto.verify === 'function') {
    const key =
      '-----BEGIN PRIVATE KEY-----\r\nMC4CAQAwBQYDK2VwBCIEIHKj+sVa9WcD'
      + '/q2DJUJaf43Kptc8xYuUQA4bOFj9vC8T\r\n-----END PRIVATE KEY-----';
    const data = Buffer.from('a');
    let sig;
    let verified;
    try {
      sig = crypto.sign(null, data, key);
      verified = crypto.verify(null, data, key, sig);
    } catch {}
    return (Buffer.isBuffer(sig) && sig.length === 64 && verified === true);
  }

  return false;
})();

You'll note that this code is run automatically when the ssh2 module is imported, so I don't see how I can circumvent it.

I am able to reproduce this on Mac OS but the issue was initially brought to my attention via testcontainers/testcontainers-node#346 where people on Linux experience the same.

I've tried searching for more information about SIGNREQUEST but cannot find anything of interest in this repository's history/issues or on Google.

Any help/suggestions would be much appreciated!

I am using version 1.4.0 of ssh2, though I see the code for eddsaSupported is the same in the latest.

[cristianrgreco]

I've noticed in the documentation for crypto.sign that if a callback is provided, the code is executed on libuv's thread pool, and trying it out it seems it fixes the problem (no open handles):

// before
sig = crypto.sign(null, data, key);

// after
crypto.sign(null, data, key (err, sig) => {
}

Because the eddsaSupported is an IIFE and evaluates to a value, changing it to a callback/promise is more involved, but just FYI the solution might be using the alternate signature.

If you have any suggestions, such as eddsaSupported can be made to return a Promise, then I'd be happy to raise a PR.

[mscdex]

This seems like a Jest bug to me because this is a synchronous API call. Either their method of detecting handles is buggy or node is doing something wrong (unlikely).

[cristianrgreco]

Thanks for the quick response @mscdex. I have raised an issue with Jest: jestjs/jest#12775

[cristianrgreco]

The PR for jest has been accepted and merged. Thanks again @mscdex