Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VSCode 1.54.1 breaks spawned processes with cwd option #118267

Closed
jeremie-stripe opened this issue Mar 5, 2021 · 14 comments
Closed

VSCode 1.54.1 breaks spawned processes with cwd option #118267

jeremie-stripe opened this issue Mar 5, 2021 · 14 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug confirmed Issue has been confirmed by VS Code Team member insiders-released Patch has been released in VS Code Insiders macos-big-sur verified Verification succeeded
Milestone

Comments

@jeremie-stripe
Copy link

  • VS Code Version: 1.54.1
  • OS Version: macOS 10.15.17

Steps to Reproduce:

  1. Upgrade to VSCode 1.54.1 from 1.53
  2. A few of our extensions are failing to start because spawned shell script processes do not work

Does this issue occur when all extensions are disabled?: No

We run a few extensions (including an LSP server) which essentially spawn an external process executing shell wrapper scripts and attempts to manipulate its standard IO.

Since upgrading to 1.54.1 we have noticed a strange behavior where when writing to the external process stdin we get EBADF error. This seems specifically to only occur when attempting to execute shell script directly as when we modify the spawn (or similar functions) to invoke the script via bash directly it works fine.

This is illustrated by the following test cases executed while debugging VSCode:

> cp.execFile("cat", ['-'], options, cb).stdin.write("foo\n")
true
> cp.execFile("scripts/bin/wrapper.sh", ["cat", "-"], options, cb).stdin.write("foo\n")
Uncaught Error: write EBADF
> cp.execFile("bash", ["scripts/bin/wrapper.sh", "cat", "-"], options, cb).stdin.write("foo\n")
true

Where wrapper.sh ultimately will exec the rest of the command line.

@weinand weinand removed their assignment Mar 6, 2021
@deepak1556 deepak1556 added bug Issue identified by VS Code Team member as probable bug confirmed Issue has been confirmed by VS Code Team member macos-big-sur labels Mar 10, 2021
@deepak1556 deepak1556 added the candidate Issue identified as probable candidate for fixing in the next release label Mar 10, 2021
@deepak1556
Copy link
Collaborator

We just released an update to VS Code Insiders that has a fix for this issue. Please try it out and let us know.

@jeremie-stripe
Copy link
Author

jeremie-stripe commented Mar 11, 2021

@deepak1556 thanks

I tested with:

Version: 1.55.0-insider (Universal)
Commit: 9b2ee7f
Date: 2021-03-11T05:15:23.749Z
Electron: 11.3.0
Chrome: 87.0.4280.141
Node.js: 12.18.3
V8: 8.7.220.31-electron.0
OS: Darwin x64 20.3.0

And the problem persists. With my mentioned test case I'm now getting EPIPE instead of EBADF:

require('child_process').execFile("bash", ["scripts/bin/wrapper.sh", "cat", "-"]).stdin.write("foo\n")
true
require('child_process').execFile("scripts/bin/wrapper.sh", ["cat", "-"]).stdin.write("foo\n")
Uncaught Error: write EPIPE

@jeremie-stripe
Copy link
Author

jeremie-stripe commented Mar 11, 2021

I tested this a bit more and another variant that I have found work is when you now specify the full path to the script (despite the cwd option being set correctly).

If the full path is not specified I will get ENOENT as if the file didn't exist (maybe cwd is not propagated properly)?

@deepak1556 deepak1556 reopened this Mar 11, 2021
@deepak1556 deepak1556 removed candidate Issue identified as probable candidate for fixing in the next release confirmed Issue has been confirmed by VS Code Team member labels Mar 11, 2021
@jeremie-stripe
Copy link
Author

jeremie-stripe commented Mar 11, 2021

To be more descriptive here is a test case:

Given this foo.sh file (marked as executable) that is saved at /Users/jeremie/stripe/sandbox/foo.sh:

#!/bin/bash

echo "called" >> /tmp/debugout

Trying to execute the following statements in the debug console while debugging VSCode insiders (same version as noted above):

→ cp.execFileSync('foo.sh', [], {cwd: '/Users/jeremie/stripe/sandbox/'})
Uncaught Error: spawnSync foo.sh ENOENT
→ cp.execFileSync('/Users/jeremie/stripe/sandbox/foo.sh', [], {cwd: '/Users/jeremie/stripe/sandbox/'})
Uint8Array(0) []
→ cp.execFileSync('/Users/jeremie/stripe/sandbox/foo.sh', [])
Uint8Array(0) []

@deepak1556
Copy link
Collaborator

deepak1556 commented Mar 11, 2021

Thanks for the test case,

@jeremie-stripe I tried your test case with node 15.9.0 and they seem to have identical results

$ node
Welcome to Node.js v15.9.0.
Type ".help" for more information.
> const cp = require('child_process')
undefined
> cp.execFileSync('foo.sh', [], {cwd: '/Users/demohan'})
Uncaught:
<ref *1> Error: spawnSync foo.sh ENOENT
    at __node_internal_captureLargerStackTrace (node:internal/errors:415:5)
    at __node_internal_errnoException (node:internal/errors:545:12)
    at Object.spawnSync (node:internal/child_process:1086:20)
    at spawnSync (node:child_process:666:24)
    at Object.execFileSync (node:child_process:693:15) {
  errno: -2,
  code: 'ENOENT',
  syscall: 'spawnSync foo.sh',
  path: 'foo.sh',
  spawnargs: [],
  error: [Circular *1],
  status: null,
  signal: null,
  output: null,
  pid: 28568,
  stdout: null,
  stderr: null
}
> cp.execFileSync('/Users/demohan/foo.sh', [], {cwd: '/Users/demohan/'})
<Buffer >

@deepak1556 deepak1556 changed the title VSCode 1.54.1 breaks spawned processes that execute shell scripts VSCode 1.54.1 breaks spawned processes with cwd option Mar 11, 2021
@deepak1556 deepak1556 added confirmed Issue has been confirmed by VS Code Team member and removed confirmed Issue has been confirmed by VS Code Team member labels Mar 11, 2021
@deepak1556 deepak1556 added confirmed Issue has been confirmed by VS Code Team member and removed confirmation-pending labels Mar 11, 2021
@jeremie-stripe
Copy link
Author

And I think more specifically our case is the following because we use subpaths to reference our scripts:

Node 10.16.0:

> cp.execFileSync('sandbox/foo.sh', [], {cwd: '/Users/jeremie/stripe/'})
<Buffer >

VSCode Insiders debug console:

> cp.execFileSync('sandbox/foo.sh', [], {cwd: '/Users/jeremie/stripe/'})
Uncaught Error: spawnSync sandbox/foo.sh ENOENT

@deepak1556
Copy link
Collaborator

Thanks for clarifying, can confirm the regression.

@jvilk-stripe
Copy link

Looking forward to seeing this bug fixed! This bug has Stripe engineering stuck on 1.53 since it impacts so many of our extensions and configurations; one common pattern in our setup is using shell scripts to wrap other tools to set them up properly for our dev environment.

@deepak1556
Copy link
Collaborator

@bpasero bpasero added the verified Verification succeeded label Apr 29, 2021
@deepak1556
Copy link
Collaborator

Fix didn't account for some edge cases on the failure path and introduced regression #122661. Reopening for next iteration.

@deepak1556 deepak1556 reopened this Apr 29, 2021
@deepak1556 deepak1556 removed insiders-released Patch has been released in VS Code Insiders verified Verification succeeded labels Apr 29, 2021
@deepak1556 deepak1556 modified the milestones: April 2021, May 2021 Apr 29, 2021
@deepak1556
Copy link
Collaborator

This turned out to be a bug in the posix_spawnp implementation where a relative executable path will cause it to successfully launch the program, but erroneously return ENOENT when used with posix_spawn_file_actions_addchdir_np because of the stat call performed on the executable path after the child process is launched. Ref: https://opensource.apple.com/source/Libc/Libc-1439.40.11/sys/posix_spawn.c.auto.html

Have resorted to use pthread_chdir_np instead to respect the cwd option, will update here once the updated runtime is available.

@jeremie-stripe
Copy link
Author

@deepak1556 thanks for figuring it out! I just tested with the latest insiders version (version info below) and I can confirm that our workloads are now properly working again!

Version: 1.57.0-insider (Universal)
Commit: 5246162662ffa9f16a70dc2b94f13b0d15511e64
Date: 2021-05-14T08:49:16.464Z
Electron: 12.0.7
Chrome: 89.0.4389.128
Node.js: 14.16.0
V8: 8.9.255.25-electron.0
OS: Darwin x64 20.4.0

@mjbvz mjbvz added the verified Verification succeeded label Jun 4, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jun 28, 2021
Charles-Gagnon pushed a commit to microsoft/azuredatastudio that referenced this issue Jul 1, 2021
Charles-Gagnon pushed a commit to microsoft/azuredatastudio that referenced this issue Jul 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug confirmed Issue has been confirmed by VS Code Team member insiders-released Patch has been released in VS Code Insiders macos-big-sur verified Verification succeeded
Projects
Archived in project
Development

No branches or pull requests

7 participants
@bpasero @deepak1556 @weinand @mjbvz @jvilk-stripe @jeremie-stripe and others