From cda21c19a764f99e2d9c4d4aa555b066a8e6b24b Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Mon, 1 May 2023 14:58:25 -0700 Subject: [PATCH] cli: fix attach does not always work (#181273) * fix: don't sync debug.lastExtensionDevelopmentWorkspace * cli: fix attach does not always work Seems like reading stdin when it's open but never written to blocks the process. Fix that, both by checking IS_INTERACTIVE_CLI before reading stdin, and by not passing stdin to the tunnel subprocess. Fixes #179122 --- cli/src/tunnels/singleton_client.rs | 42 ++++++++++--------- src/vs/code/node/cli.ts | 13 +++--- .../remoteTunnel/node/remoteTunnelService.ts | 8 ++-- 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/cli/src/tunnels/singleton_client.rs b/cli/src/tunnels/singleton_client.rs index 98d2ea86e1f..d431d9d7a06 100644 --- a/cli/src/tunnels/singleton_client.rs +++ b/cli/src/tunnels/singleton_client.rs @@ -63,29 +63,31 @@ pub async fn start_singleton_client(args: SingletonClientArgs) -> bool { "An existing tunnel is running on this machine, connecting to it..." ); - let stdin_handle = rpc.get_caller(msg_tx.clone()); - thread::spawn(move || { - let mut input = String::new(); - loop { - input.truncate(0); + if *IS_INTERACTIVE_CLI { + let stdin_handle = rpc.get_caller(msg_tx.clone()); + thread::spawn(move || { + let mut input = String::new(); + loop { + input.truncate(0); + println!("reading line"); + match std::io::stdin().read_line(&mut input) { + Err(_) | Ok(0) => return, // EOF or not a tty + _ => {} + }; - match std::io::stdin().read_line(&mut input) { - Err(_) | Ok(0) => return, // EOF or not a tty - _ => {} - }; - - match input.chars().next().map(|c| c.to_ascii_lowercase()) { - Some('x') => { - stdin_handle.notify(protocol::singleton::METHOD_SHUTDOWN, EmptyObject {}); - return; + match input.chars().next().map(|c| c.to_ascii_lowercase()) { + Some('x') => { + stdin_handle.notify(protocol::singleton::METHOD_SHUTDOWN, EmptyObject {}); + return; + } + Some('r') => { + stdin_handle.notify(protocol::singleton::METHOD_RESTART, EmptyObject {}); + } + Some(_) | None => {} } - Some('r') => { - stdin_handle.notify(protocol::singleton::METHOD_RESTART, EmptyObject {}); - } - Some(_) | None => {} } - } - }); + }); + } let caller = rpc.get_caller(msg_tx); let mut rpc = rpc.methods(SingletonServerContext { diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index ecec7074911..931ca9a2814 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { ChildProcess, ChildProcessWithoutNullStreams, spawn, SpawnOptions } from 'child_process'; +import { ChildProcess, spawn, SpawnOptions, StdioOptions } from 'child_process'; import { chmodSync, existsSync, readFileSync, statSync, truncateSync, unlinkSync } from 'fs'; import { homedir, release, tmpdir } from 'os'; import type { ProfilingSession, Target } from 'v8-inspect-profiler'; @@ -56,20 +56,21 @@ export async function main(argv: string[]): Promise { } const tunnelArgs = argv.slice(argv.indexOf('tunnel') + 1); // all arguments behind `tunnel` return new Promise((resolve, reject) => { - let tunnelProcess: ChildProcessWithoutNullStreams; + let tunnelProcess: ChildProcess; + const stdio: StdioOptions = ['ignore', 'pipe', 'pipe']; if (process.env['VSCODE_DEV']) { - tunnelProcess = spawn('cargo', ['run', '--', 'tunnel', ...tunnelArgs], { cwd: join(getAppRoot(), 'cli') }); + tunnelProcess = spawn('cargo', ['run', '--', 'tunnel', ...tunnelArgs], { cwd: join(getAppRoot(), 'cli'), stdio }); } else { const appPath = process.platform === 'darwin' // ./Contents/MacOS/Electron => ./Contents/Resources/app/bin/code-tunnel-insiders ? join(dirname(dirname(process.execPath)), 'Resources', 'app') : dirname(process.execPath); const tunnelCommand = join(appPath, 'bin', `${product.tunnelApplicationName}${isWindows ? '.exe' : ''}`); - tunnelProcess = spawn(tunnelCommand, ['tunnel', ...tunnelArgs], { cwd: cwd() }); + tunnelProcess = spawn(tunnelCommand, ['tunnel', ...tunnelArgs], { cwd: cwd(), stdio }); } - tunnelProcess.stdout.pipe(process.stdout); - tunnelProcess.stderr.pipe(process.stderr); + tunnelProcess.stdout!.pipe(process.stdout); + tunnelProcess.stderr!.pipe(process.stderr); tunnelProcess.on('exit', resolve); tunnelProcess.on('error', reject); }); diff --git a/src/vs/platform/remoteTunnel/node/remoteTunnelService.ts b/src/vs/platform/remoteTunnel/node/remoteTunnelService.ts index 00fbdcc2dea..9b73eece6ba 100644 --- a/src/vs/platform/remoteTunnel/node/remoteTunnelService.ts +++ b/src/vs/platform/remoteTunnel/node/remoteTunnelService.ts @@ -10,7 +10,7 @@ import { INativeEnvironmentService } from 'vs/platform/environment/common/enviro import { Disposable } from 'vs/base/common/lifecycle'; import { ILogger, ILoggerService, LogLevelToString } from 'vs/platform/log/common/log'; import { dirname, join } from 'vs/base/common/path'; -import { ChildProcess, spawn } from 'child_process'; +import { ChildProcess, StdioOptions, spawn } from 'child_process'; import { IProductService } from 'vs/platform/product/common/productService'; import { isMacintosh, isWindows } from 'vs/base/common/platform'; import { CancelablePromise, createCancelablePromise, Delayer } from 'vs/base/common/async'; @@ -322,6 +322,8 @@ export class RemoteTunnelService extends Disposable implements IRemoteTunnelServ resolve(-1); } let tunnelProcess: ChildProcess | undefined; + const stdio: StdioOptions = ['ignore', 'pipe', 'pipe']; + token.onCancellationRequested(() => { if (tunnelProcess) { this._logger.info(`${logLabel} terminating(${tunnelProcess.pid})`); @@ -331,12 +333,12 @@ export class RemoteTunnelService extends Disposable implements IRemoteTunnelServ if (!this.environmentService.isBuilt) { onOutput('Building tunnel CLI from sources and run', false); onOutput(`${logLabel} Spawning: cargo run -- tunnel ${commandArgs.join(' ')}`, false); - tunnelProcess = spawn('cargo', ['run', '--', 'tunnel', ...commandArgs], { cwd: join(this.environmentService.appRoot, 'cli') }); + tunnelProcess = spawn('cargo', ['run', '--', 'tunnel', ...commandArgs], { cwd: join(this.environmentService.appRoot, 'cli'), stdio }); } else { onOutput('Running tunnel CLI', false); const tunnelCommand = this.getTunnelCommandLocation(); onOutput(`${logLabel} Spawning: ${tunnelCommand} tunnel ${commandArgs.join(' ')}`, false); - tunnelProcess = spawn(tunnelCommand, ['tunnel', ...commandArgs], { cwd: homedir() }); + tunnelProcess = spawn(tunnelCommand, ['tunnel', ...commandArgs], { cwd: homedir(), stdio }); } tunnelProcess.stdout!.on('data', data => {