Clean-up child processes on dispose

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<number>;
-    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<void> {
-        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');

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

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<string, LanguageClient> = new Map();
@@ -29,9 +28,9 @@ export class LanguageService implements Disposable {
     let client: LanguageClient;
 
     const debug = config.get<boolean>('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<boolean>('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<string[]>('lsp.args').concat('--quiet', '--slave');
+    const initArgs: string[] = config.get<string[]>('lsp.args').concat('--silent', '--slave');
 
-    const tcpServerOptions = () => new Promise<ChildProcess | StreamInfo>((resolve, reject) => {
+    const tcpServerOptions = () => new Promise<DisposableProcess | StreamInfo>((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);
     }

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<KnitWorkingDirectory>('rmarkdown.knit.defaults.knitWorkingDirectory') ?? undefined;
 

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%'

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<void> {
         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(() => {

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<T>(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<string>, 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;
+}