diff --git a/src/helpViewer/helpProvider.ts b/src/helpViewer/helpProvider.ts index 5ca646085..8a9531df9 100644 --- a/src/helpViewer/helpProvider.ts +++ b/src/helpViewer/helpProvider.ts @@ -2,13 +2,13 @@ import { Memento, window } from 'vscode'; import * as http from 'http'; import * as cp from 'child_process'; -import * as kill from 'tree-kill'; import * as path from 'path'; import * as fs from 'fs'; import * as os from 'os'; import * as rHelp from '.'; import { extensionContext } from '../extension'; +import { DisposableProcess, exec } from '../util'; export interface RHelpProviderOptions { // path of the R executable @@ -19,9 +19,8 @@ export interface RHelpProviderOptions { pkgListener?: () => void; } -type ChildProcessWithPort = cp.ChildProcess & { +type ChildProcessWithPort = DisposableProcess & { port?: number | Promise; - running?: boolean; }; // Class to forward help requests to a backgorund R instance that is running a help server @@ -39,10 +38,7 @@ export class HelpProvider { } public async refresh(): Promise { - if (this.cp.running) { - // this.cp.kill('SIGKILL'); - kill(this.cp.pid, 'SIGKILL'); // more reliable than cp.kill (?) - } + this.cp.dispose(); this.cp = this.launchRHelpServer(); await this.cp.port; } @@ -55,17 +51,21 @@ export class HelpProvider { // starts the background help server and waits forever to keep the R process running const scriptPath = extensionContext.asAbsolutePath('R/help/helpServer.R'); - const cmd = ( - `${this.rPath} --silent --slave --no-save --no-restore -f ` + - `${scriptPath}` - ); + // const cmd = `${this.rPath} --silent --slave --no-save --no-restore -f "${scriptPath}"`; + const args = [ + '--slient', + '--slave', + '--no-save', + '--no-restore', + '-f', + scriptPath + ]; const cpOptions = { cwd: this.cwd, - env: { ...process.env, 'VSCR_LIM': lim } + env: { ...process.env, 'VSCR_LIM': lim }, }; - const childProcess: ChildProcessWithPort = cp.exec(cmd, cpOptions); - childProcess.running = true; + const childProcess: ChildProcessWithPort = exec(this.rPath, args, cpOptions); let str = ''; // promise containing the port number of the process (or 0) @@ -93,7 +93,6 @@ export class HelpProvider { const exitHandler = () => { childProcess.port = 0; - childProcess.running = false; }; childProcess.on('exit', exitHandler); childProcess.on('error', exitHandler); @@ -174,10 +173,7 @@ export class HelpProvider { dispose(): void { - if (this.cp.running) { - // this.cp.kill('SIGKILL'); - kill(this.cp.pid, 'SIGKILL'); - } + this.cp.dispose(); } } @@ -281,13 +277,13 @@ export class AliasProvider { const re = new RegExp(`^.*?${lim}(.*)${lim}.*$`, 'ms'); const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'vscode-R-aliases')); const tempFile = path.join(tempDir, 'aliases.json'); - const cmd = `${this.rPath} --silent --no-save --no-restore --slave -f "${this.rScriptFile}" > "${tempFile}"`; + const cmd = `"${this.rPath}" --silent --no-save --no-restore --slave -f "${this.rScriptFile}" > "${tempFile}"`; let allPackageAliases: undefined | AllPackageAliases = undefined; try{ // execute R script 'getAliases.R' // aliases will be written to tempFile - cp.execSync(cmd, {cwd: this.cwd}); + cp.execSync(cmd, { cwd: this.cwd }); // read and parse aliases const txt = fs.readFileSync(tempFile, 'utf-8'); diff --git a/src/helpViewer/index.ts b/src/helpViewer/index.ts index fd2cfec7d..0f4913e98 100644 --- a/src/helpViewer/index.ts +++ b/src/helpViewer/index.ts @@ -29,7 +29,7 @@ export async function initializeHelp( void vscode.commands.executeCommand('setContext', 'r.helpViewer.show', true); // get the "vanilla" R path from config - const rPath = await getRpath(true); + const rPath = await getRpath(false); // get the current working directory from vscode const cwd = vscode.workspace.workspaceFolders?.length diff --git a/src/languageService.ts b/src/languageService.ts index 3ce3ec151..8e3114529 100644 --- a/src/languageService.ts +++ b/src/languageService.ts @@ -7,10 +7,9 @@ import os = require('os'); import path = require('path'); import net = require('net'); import url = require('url'); -import { spawn, ChildProcess } from 'child_process'; import { LanguageClient, LanguageClientOptions, StreamInfo, DocumentFilter, ErrorAction, CloseAction, RevealOutputChannelOn } from 'vscode-languageclient/node'; import { Disposable, workspace, Uri, TextDocument, WorkspaceConfiguration, OutputChannel, window, WorkspaceFolder } from 'vscode'; -import { getRpath } from './util'; +import { DisposableProcess, getRpath, exec } from './util'; export class LanguageService implements Disposable { private readonly clients: Map = new Map(); @@ -29,9 +28,9 @@ export class LanguageService implements Disposable { let client: LanguageClient; const debug = config.get('lsp.debug'); - const path = await getRpath(); + const rPath = await getRpath(); if (debug) { - console.log(`R binary: ${path}`); + console.log(`R path: ${rPath}`); } const use_stdio = config.get('lsp.use_stdio'); const env = Object.create(process.env); @@ -46,9 +45,9 @@ export class LanguageService implements Disposable { } const options = { cwd: cwd, env: env }; - const initArgs: string[] = config.get('lsp.args').concat('--quiet', '--slave'); + const initArgs: string[] = config.get('lsp.args').concat('--silent', '--slave'); - const tcpServerOptions = () => new Promise((resolve, reject) => { + const tcpServerOptions = () => new Promise((resolve, reject) => { // Use a TCP socket because of problems with blocking STDIO const server = net.createServer(socket => { // 'connection' listener @@ -66,14 +65,10 @@ export class LanguageService implements Disposable { // Listen on random port server.listen(0, '127.0.0.1', () => { const port = (server.address() as net.AddressInfo).port; - let args: string[]; - // The server is implemented in R - if (debug) { - args = initArgs.concat(['-e', `languageserver::run(port=${port},debug=TRUE)`]); - } else { - args = initArgs.concat(['-e', `languageserver::run(port=${port})`]); - } - const childProcess = spawn(path, args, options); + const expr = debug ? `languageserver::run(port=${port},debug=TRUE)` : `languageserver::run(port=${port})`; + // const cmd = `${rPath} ${initArgs.join(' ')} -e "${expr}"`; + const args = initArgs.concat(['-e', expr]); + const childProcess = exec(rPath, args, options); client.outputChannel.appendLine(`R Language Server (${childProcess.pid}) started`); childProcess.stderr.on('data', (chunk: Buffer) => { client.outputChannel.appendLine(chunk.toString()); @@ -122,7 +117,7 @@ export class LanguageService implements Disposable { } else { args = initArgs.concat(['-e', `languageserver::run()`]); } - client = new LanguageClient('r', 'R Language Server', { command: path, args: args, options: options }, clientOptions); + client = new LanguageClient('r', 'R Language Server', { command: rPath, args: args, options: options }, clientOptions); } else { client = new LanguageClient('r', 'R Language Server', tcpServerOptions, clientOptions); } diff --git a/src/rmarkdown/knit.ts b/src/rmarkdown/knit.ts index 5c28ebc65..5b8c2c78b 100644 --- a/src/rmarkdown/knit.ts +++ b/src/rmarkdown/knit.ts @@ -4,9 +4,10 @@ import * as fs from 'fs-extra'; import path = require('path'); import yaml = require('js-yaml'); -import { RMarkdownManager, KnitWorkingDirectory, DisposableProcess } from './manager'; +import { RMarkdownManager, KnitWorkingDirectory } from './manager'; import { runTextInTerm } from '../rTerminal'; import { extensionContext, rmdPreviewManager } from '../extension'; +import { DisposableProcess } from '../util'; export let knitDir: KnitWorkingDirectory = util.config().get('rmarkdown.knit.defaults.knitWorkingDirectory') ?? undefined; diff --git a/src/rmarkdown/manager.ts b/src/rmarkdown/manager.ts index de4f761e8..102b26237 100644 --- a/src/rmarkdown/manager.ts +++ b/src/rmarkdown/manager.ts @@ -2,14 +2,13 @@ import * as util from '../util'; import * as vscode from 'vscode'; import * as cp from 'child_process'; import path = require('path'); +import { DisposableProcess, exec } from '../util'; export enum KnitWorkingDirectory { documentDirectory = 'document directory', workspaceRoot = 'workspace root', } -export type DisposableProcess = cp.ChildProcessWithoutNullStreams & vscode.Disposable; - export interface IKnitRejection { cp: DisposableProcess; wasCancelled: boolean; @@ -66,14 +65,14 @@ export abstract class RMarkdownManager { const scriptArgs = args.scriptArgs; const scriptPath = args.scriptPath; const fileName = args.fileName; - - const processArgs = [ - `--silent`, - `--slave`, - `--no-save`, - `--no-restore`, - `-f`, - `${scriptPath}` + // const cmd = `${this.rPath} --silent --slave --no-save --no-restore -f "${scriptPath}"`; + const cpArgs = [ + '--slient', + '--slave', + '--no-save', + '--no-restore', + '-f', + scriptPath ]; const processOptions: cp.SpawnOptions = { env: { @@ -84,21 +83,11 @@ export abstract class RMarkdownManager { }; let childProcess: DisposableProcess; - try { - childProcess = util.asDisposable( - cp.spawn( - `${this.rPath}`, - processArgs, - processOptions - ), - () => { - if (childProcess.kill('SIGKILL')) { - rMarkdownOutput.appendLine('[VSC-R] terminating R process'); - printOutput = false; - } - } - ); + childProcess = exec(this.rPath, cpArgs, processOptions, () => { + rMarkdownOutput.appendLine('[VSC-R] terminating R process'); + printOutput = false; + }); progress.report({ increment: 0, message: '0%' diff --git a/src/rmarkdown/preview.ts b/src/rmarkdown/preview.ts index 8d972334d..265bad40d 100644 --- a/src/rmarkdown/preview.ts +++ b/src/rmarkdown/preview.ts @@ -6,10 +6,10 @@ import path = require('path'); import crypto = require('crypto'); -import { config, readContent, setContext, escapeHtml, UriIcon, saveDocument, getRpath } from '../util'; +import { config, readContent, setContext, escapeHtml, UriIcon, saveDocument, getRpath, DisposableProcess } from '../util'; import { extensionContext, tmpDir } from '../extension'; import { knitDir } from './knit'; -import { DisposableProcess, RMarkdownManager } from './manager'; +import { RMarkdownManager } from './manager'; class RMarkdownPreview extends vscode.Disposable { title: string; @@ -27,7 +27,7 @@ class RMarkdownPreview extends vscode.Disposable { resourceViewColumn: vscode.ViewColumn, outputUri: vscode.Uri, filePath: string, RMarkdownPreviewManager: RMarkdownPreviewManager, useCodeTheme: boolean, autoRefresh: boolean) { super(() => { - this.cp?.kill('SIGKILL'); + this.cp?.dispose(); this.panel?.dispose(); this.fileWatcher?.close(); fs.removeSync(this.outputUri.fsPath); @@ -271,7 +271,7 @@ export class RMarkdownPreviewManager extends RMarkdownManager { public async updatePreview(preview?: RMarkdownPreview): Promise { const toUpdate = preview ?? this.activePreview?.preview; const previewUri = this.previewStore?.getFilePath(toUpdate); - toUpdate?.cp?.kill('SIGKILL'); + toUpdate?.cp?.dispose(); if (toUpdate) { const childProcess: DisposableProcess | void = await this.previewDocument(previewUri, toUpdate.title).catch(() => { diff --git a/src/util.ts b/src/util.ts index 8348ae58c..c5e568984 100644 --- a/src/util.ts +++ b/src/util.ts @@ -8,6 +8,7 @@ import * as vscode from 'vscode'; import * as cp from 'child_process'; import { rGuestService, isGuestSession } from './liveShare'; import { extensionContext } from './extension'; +import * as kill from 'tree-kill'; export function config(): vscode.WorkspaceConfiguration { return vscode.workspace.getConfiguration('r'); @@ -407,3 +408,33 @@ export function asDisposable(toDispose: T, disposeFunction: (...args: unknown extensionContext.subscriptions.push(toDispose as disposeType); return toDispose as disposeType; } + +export type DisposableProcess = cp.ChildProcessWithoutNullStreams & vscode.Disposable; +export function exec(command: string, args?: ReadonlyArray, options?: cp.CommonOptions, onDisposed?: () => unknown): DisposableProcess { + let proc: cp.ChildProcess; + if (process.platform === 'win32') { + const cmd = `"${command}" ${args.map(s => `"${s}"`).join(' ')}`; + proc = cp.exec(cmd, options); + } else { + proc = cp.spawn(command, args, options); + } + let running = true; + const exitHandler = () => { + running = false; + }; + proc.on('exit', exitHandler); + proc.on('error', exitHandler); + const disposable = asDisposable(proc, () => { + if (running) { + if (process.platform === 'win32') { + kill(proc.pid); + } else { + proc.kill('SIGKILL'); + } + } + if (onDisposed) { + onDisposed(); + } + }); + return disposable; +}