`ruby-lsp` hangs `vscode-server` during version manager detection

Description

Ruby LSP Information

VS Code Version

1.137.0

Ruby LSP Extension Version

0.10.6

Ruby LSP Server Version

0.26.11

Ruby LSP Add-ons

Ruby Version

4.0.7

Ruby Version Manager

rbenv

Installed Extensions

Ruby LSP Settings

{}

{
  "enabledFeatures": {
    "codeActions": true,
    "diagnostics": true,
    "documentHighlights": true,
    "documentLink": true,
    "documentSymbols": true,
    "foldingRanges": true,
    "formatting": true,
    "hover": true,
    "inlayHint": true,
    "onTypeFormatting": true,
    "selectionRanges": true,
    "semanticHighlighting": true,
    "completion": true,
    "codeLens": true,
    "definition": true,
    "workspaceSymbol": true,
    "signatureHelp": true,
    "typeHierarchy": true
  },
  "featuresConfiguration": {},
  "addonSettings": {},
  "rubyVersionManager": {
    "identifier": "auto"
  },
  "customRubyCommand": "",
  "formatter": "auto",
  "linters": null,
  "bundleGemfile": "",
  "testTimeout": 30,
  "pullDiagnosticsOn": "both",
  "useBundlerCompose": false,
  "bypassTypechecker": false,
  "rubyExecutablePath": "",
  "indexing": {},
  "erbSupport": true,
  "featureFlags": {},
  "sigOpacityLevel": "1"
}

TLDR:

asyncExec can hang the extension (and vscode-server) when it makes multiple interactive shell calls (e.g. during version manager detection). spawn is probably a more resilient strategy.

Details

During initial version manager detection in a remote workspace, and when vscode.env.shell is set, Ruby.toolExists causes the plugin (and in fact the whole process tree) to hang.

For example, when vscode.env.shell is /bin/bash, the command passed to asyncExec looks like: /bin/bash -i -c 'rbenv --version'
Because Node's default shell is /bin/sh, this effectively invokes: /bin/sh -c /bin/bash -i -c 'rbenv --version'

USER       PID TTY  STAT COMMAND
paul     12933 tty2 T     \_ sh -c "$VSCODE_WSL_EXT_LOCATION/scripts/wslServer.sh" 645f29cc3176500b4b5762ba887cf2a7f0ffdf2c stable code-server .vscode-server
paul     12934 tty2 T         \_ sh /mnt/c/Users/Paul/.vscode/extensions/ms-vscode-remote.remote-wsl-0.104.3/scripts/wslServer.sh 645f29cc3176500b4b5762ba887cf2a7f0ffdf2c
paul     12940 tty2 T             \_ sh /home/paul/.vscode-server/bin/645f29cc3176500b4b5762ba887cf2a7f0ffdf2c/bin/code-server
paul     12944 tty2 Tl                \_ /home/paul/.vscode-server/bin/645f29cc3176500b4b5762ba887cf2a7f0ffdf2c/node /home/paul/.vscode-server/bin/645f29cc3176500b4b5762ba887cf2a7f0ffdf2c/out/server-main.js
paul     12965 tty2 Tl                    \_ /home/paul/.vscode-server/bin/645f29cc3176500b4b5762ba887cf2a7f0ffdf2c/node /home/paul/.vscode-server/bin/645f29cc3176500b4b5762ba887cf2a7f0ffdf2c/out/bootstrap-fork
paul     13023 tty2 Tl                    \_ /home/paul/.vscode-server/bin/645f29cc3176500b4b5762ba887cf2a7f0ffdf2c/node --dns-result-order=ipv4first /home/paul/.vscode-server/bin/645f29cc3176500b4b5762ba887cf2a7f0ffdf2c/out/bootstrap-fork
paul     13189 tty2 T                         \_ /bin/sh -c /bin/bash -i -c 'rbenv --version'
paul     13190 tty2 T                             \_ /bin/bash -i -c rbenv --version

Now, initially, I thought this was a quoting issue -- at first glance it appears as if the -c argument is not properly quoted because of the double-shell. But /proc/<pid>/cmdline confirms the arguments are indeed being grouped correctly. (e.g. cat /proc/13189/cmdline | tr '\0' '\n').

What is actually happening here is a job control issue due to the use of -i. Note the T (stopped) flag in the process status.

From what I understand, when bash is invoked interactively, it manipulates the controlling terminal's foreground process (tcsetpgrp against tty2 above). If the extension is checking for multiple version managers (say chruby, then rbenv), the first invocation and exit of bash -i effectively taints the job control state of the process group because once it exits, that TTY's registered foreground process is now dead -- and the kernel's response to subsequent job control calls in that terminal is to suspend the whole calling process group. This causes both the extension and VS Code server to become unresponsive.

Per tcsetpgrp:

If tcsetpgrp() is called by a member of a background process group in its session, and the calling process is not blocking
or ignoring SIGTTOU, a SIGTTOU signal is sent to all members of this background process group.

I'm guessing we use -i because it's desirable to source user profiles to make version managers available. So, I believe the correct fix for this is to use child_process.spawn which, unlike exec, has a detached option for running the child process independently from the parent. With that, subprocesses won't interfere with the state of the whole process group.

Furthermore -- the nested shell calling may be unnecessary. The shell option can be used to invoke the user's shell directly instead of invoking it under /bin/sh.

Reproduction steps

PoC script replicating the behavior:

const { exec } = require('child_process');
const { promisify } = require('util');
const asyncExec = promisify(exec);

const userShell = `/bin/bash`;

(async function() {
  async function run(cmd) {
    let out;
    try {
      out = await asyncExec(`${userShell} -i -c '${cmd}'`, {
        shell: '/bin/sh'
      });
    } catch (e) {
      out = `[stderr] ${e.message}`;
    }

    console.log(cmd, out);
  }

  await run('chruby --version');
  await run('rbenv --version');
  await run('rvm --version');
})();

Resulting output:

$ node test.js
chruby --version [stderr] Command failed: /bin/bash -i -c 'chruby --version'
chruby: command not found


[1]+  Stopped                 node test.js

$ fg
node test.js
rbenv --version [stderr] Command failed: /bin/bash -i -c 'rbenv --version'
Command 'rbenv' not found, but can be installed with:
sudo apt install rbenv


[1]+  Stopped                 node test.js

$ fg
node test.js
rvm --version [stderr] Command failed: /bin/bash -i -c 'rvm --version'
Command 'rvm' not found, but there are 19 similar ones.

PoC for fix

Changing detached to false here replicates the same behavior as above.

const { spawn } = require('child_process');

const userShell = `/bin/bash`;

function run(cmd) {
  return new Promise((resolve, reject) => {
    // runs: [ '/bin/bash', '-i', '-c', 'chruby --version' ],
    let args = [`-i`, `-c`, cmd];
    let child = spawn(userShell, args, {
      detached: true
    });

    let hadError = false; 
    child.on('error', (e) => {
      hadError = true;
      console.log(`${cmd} got error ${e}`);
      reject(e);
    });
    child.on('exit', (code) => {
      console.log(`${cmd} child got RC ${code}`);
        
      // The 'exit' event may or may not fire after an error has occurred
      if (hadError) {
        return;
      }

      return code == 0 ? resolve() : reject();
    });
  });
}

(async function() {
  let cmds = ['chruby --version', 'rbenv --version', 'rvm --version'];

  for (let cmd of cmds) {
    try {
      await run(cmd);
      console.log(`${cmd} success`);
    } catch (e) {
      console.log(`${cmd} failed`);
    }
  }
})();

The affected entry point is vscode/src/ruby.ts, in Ruby.toolExists; start there and compare its asyncExec invocation with the supplied Node spawn PoC. Reproduce the repeated interactive-shell checks, then verify version-manager detection completes without stopping the extension or vscode-server while still finding installed managers.