Fix linux tests to report correctly, get rid of stream destroyed mess…

…ages on raw kernel shutdown (microsoft#12643)

news/3 Code Health/12539.md

@@ -0,0 +1 @@
+Fix linux nightly tests so they run and report results. Also seems to get rid of stream destroyed messages for raw kernel.

src/client/common/process/baseDaemon.ts

@@ -8,6 +8,7 @@ import * as os from 'os';
 import { Subject } from 'rxjs/Subject';
 import * as util from 'util';
 import { MessageConnection, NotificationType, RequestType, RequestType0 } from 'vscode-jsonrpc';
+import { IPlatformService } from '../../common/platform/types';
 import { traceError, traceInfo, traceVerbose, traceWarning } from '../logger';
 import { IDisposable } from '../types';
 import { createDeferred, Deferred } from '../utils/async';
@@ -49,6 +50,7 @@ export abstract class BasePythonDaemon {
     private disposed = false;
     constructor(
         protected readonly pythonExecutionService: IPythonExecutionService,
+        protected readonly platformService: IPlatformService,
         protected readonly pythonPath: string,
         public readonly proc: ChildProcess,
         public readonly connection: MessageConnection
@@ -61,15 +63,27 @@ export abstract class BasePythonDaemon {
         this.monitorConnection();
     }
     public dispose() {
-        try {
-            this.disposed = true;
-            // The daemon should die as a result of this.
-            this.connection.sendNotification(new NotificationType('exit'));
-            this.proc.kill();
-        } catch {
-            noop();
+        // Make sure that we only dispose once so we are not sending multiple kill signals or notifications
+        // This daemon can be held by multiple disposes such as a jupyter server daemon process which can
+        // be disposed by both the connection and the main async disposable
+        if (!this.disposed) {
+            try {
+                this.disposed = true;
+
+                // Proc.kill uses a 'SIGTERM' signal by default to kill. This was failing to kill the process
+                // sometimes on Mac and Linux. Changing this over to a 'SIGKILL' to fully kill the process.
+                // Windows closes with a different non-signal message, so keep that the same
+                // See kill_kernel message of kernel_launcher_daemon.py for and example of this.
+                if (this.platformService.isWindows) {
+                    this.proc.kill();
+                } else {
+                    this.proc.kill('SIGKILL');
+                }
+            } catch {
+                noop();
+            }
+            this.disposables.forEach((item) => item.dispose());
         }
-        this.disposables.forEach((item) => item.dispose());
     }
     public execObservable(args: string[], options: SpawnOptions): ObservableExecutionResult<string> {
         if (this.isAlive && this.canExecFileUsingDaemon(args, options)) {

src/client/common/process/pythonDaemon.ts

@@ -9,6 +9,7 @@ import { PythonExecInfo } from '../../pythonEnvironments/exec';
 import { InterpreterInformation } from '../../pythonEnvironments/info';
 import { extractInterpreterInfo } from '../../pythonEnvironments/info/interpreter';
 import { traceWarning } from '../logger';
+import { IPlatformService } from '../platform/types';
 import { BasePythonDaemon } from './baseDaemon';
 import { PythonEnvInfo } from './internal/scripts';
 import {
@@ -34,11 +35,12 @@ export class DaemonError extends Error {
 export class PythonDaemonExecutionService extends BasePythonDaemon implements IPythonDaemonExecutionService {
     constructor(
         pythonExecutionService: IPythonExecutionService,
+        platformService: IPlatformService,
         pythonPath: string,
         proc: ChildProcess,
         connection: MessageConnection
     ) {
-        super(pythonExecutionService, pythonPath, proc, connection);
+        super(pythonExecutionService, platformService, pythonPath, proc, connection);
     }
     public async getInterpreterInformation(): Promise<InterpreterInformation | undefined> {
         try {

src/client/common/process/pythonDaemonFactory.ts

@@ -13,6 +13,7 @@ import {
 import { EXTENSION_ROOT_DIR } from '../../constants';
 import { PYTHON_WARNINGS } from '../constants';
 import { traceDecorators, traceError } from '../logger';
+import { IPlatformService } from '../platform/types';
 import { IDisposable, IDisposableRegistry } from '../types';
 import { createDeferred } from '../utils/async';
 import { BasePythonDaemon } from './baseDaemon';
@@ -26,6 +27,7 @@ export class PythonDaemonFactory {
         protected readonly disposables: IDisposableRegistry,
         protected readonly options: DaemonExecutionFactoryCreationOptions,
         protected readonly pythonExecutionService: IPythonExecutionService,
+        protected readonly platformService: IPlatformService,
         protected readonly activatedEnvVariables?: NodeJS.ProcessEnv
     ) {
         if (!options.pythonPath) {
@@ -82,7 +84,13 @@ export class PythonDaemonFactory {
             await this.testDaemon(connection);
 
             const cls = this.options.daemonClass ?? PythonDaemonExecutionService;
-            const instance = new cls(this.pythonExecutionService, this.pythonPath, daemonProc.proc, connection);
+            const instance = new cls(
+                this.pythonExecutionService,
+                this.platformService,
+                this.pythonPath,
+                daemonProc.proc,
+                connection
+            );
             if (instance instanceof BasePythonDaemon) {
                 this.disposables.push(instance);
                 return (instance as unknown) as T;

src/client/common/process/pythonDaemonPool.ts

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+import { IPlatformService } from '../../common/platform/types';
 import { PythonExecInfo } from '../../pythonEnvironments/exec';
 import { InterpreterInformation } from '../../pythonEnvironments/info';
 import { IDisposableRegistry } from '../types';
@@ -32,10 +33,11 @@ export class PythonDaemonExecutionServicePool extends PythonDaemonFactory implem
         disposables: IDisposableRegistry,
         options: PooledDaemonExecutionFactoryCreationOptions,
         pythonExecutionService: IPythonExecutionService,
+        platformService: IPlatformService,
         activatedEnvVariables?: NodeJS.ProcessEnv,
         private readonly timeoutWaitingForDaemon: number = 1_000
     ) {
-        super(disposables, options, pythonExecutionService, activatedEnvVariables);
+        super(disposables, options, pythonExecutionService, platformService, activatedEnvVariables);
         this.disposables.push(this);
     }
     public async initialize() {

src/client/common/process/pythonExecutionFactory.ts

@@ -4,6 +4,7 @@ import { inject, injectable } from 'inversify';
 import { gte } from 'semver';
 
 import { Uri } from 'vscode';
+import { IPlatformService } from '../../common/platform/types';
 import { IEnvironmentActivationService } from '../../interpreter/activation/types';
 import { ICondaService, IInterpreterService } from '../../interpreter/contracts';
 import { IWindowsStoreInterpreter } from '../../interpreter/locators/types';
@@ -50,7 +51,8 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
         @inject(IConfigurationService) private readonly configService: IConfigurationService,
         @inject(ICondaService) private readonly condaService: ICondaService,
         @inject(IBufferDecoder) private readonly decoder: IBufferDecoder,
-        @inject(WindowsStoreInterpreter) private readonly windowsStoreInterpreter: IWindowsStoreInterpreter
+        @inject(WindowsStoreInterpreter) private readonly windowsStoreInterpreter: IWindowsStoreInterpreter,
+        @inject(IPlatformService) private readonly platformService: IPlatformService
     ) {
         // Acquire other objects here so that if we are called during dispose they are available.
         this.disposables = this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry);
@@ -109,6 +111,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
                     this.disposables,
                     { ...options, pythonPath },
                     activatedProc!,
+                    this.platformService,
                     activatedEnvVars
                 );
                 await daemon.initialize();
@@ -119,6 +122,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
                     this.disposables,
                     { ...options, pythonPath },
                     activatedProc!,
+                    this.platformService,
                     activatedEnvVars
                 );
                 return factory.createDaemonService<T>();

src/client/datascience/kernel-launcher/kernelDaemon.ts

@@ -6,6 +6,7 @@
 import { ChildProcess } from 'child_process';
 import { Subject } from 'rxjs/Subject';
 import { MessageConnection, NotificationType, RequestType, RequestType0 } from 'vscode-jsonrpc';
+import { IPlatformService } from '../../common/platform/types';
 import { BasePythonDaemon, ExecResponse } from '../../common/process/baseDaemon';
 import {
     IPythonExecutionService,
@@ -14,7 +15,6 @@ import {
     SpawnOptions,
     StdErrError
 } from '../../common/process/types';
-import { noop } from '../../common/utils/misc';
 import { IPythonKernelDaemon, PythonKernelDiedError } from './types';
 
 export class PythonKernelDaemon extends BasePythonDaemon implements IPythonKernelDaemon {
@@ -25,11 +25,12 @@ export class PythonKernelDaemon extends BasePythonDaemon implements IPythonKerne
     private readonly subject = new Subject<Output<string>>();
     constructor(
         pythonExecutionService: IPythonExecutionService,
+        platformService: IPlatformService,
         pythonPath: string,
         proc: ChildProcess,
         connection: MessageConnection
     ) {
-        super(pythonExecutionService, pythonPath, proc, connection);
+        super(pythonExecutionService, platformService, pythonPath, proc, connection);
     }
     public async interrupt() {
         const request = new RequestType0<void, void, void>('interrupt_kernel');
@@ -43,10 +44,6 @@ export class PythonKernelDaemon extends BasePythonDaemon implements IPythonKerne
         const request = new RequestType0<void, void, void>('kill_kernel');
         await this.sendRequestWithoutArgs(request);
     }
-    public dispose() {
-        this.kill().catch(noop);
-        super.dispose();
-    }
     public async preWarm() {
         if (this.started) {
             return;

src/test/common/process/pythonDaemon.functional.test.ts

@@ -18,6 +18,7 @@ import {
     StreamMessageReader,
     StreamMessageWriter
 } from 'vscode-jsonrpc/node';
+import { IPlatformService } from '../../../client/common/platform/types';
 import { PythonDaemonExecutionService } from '../../../client/common/process/pythonDaemon';
 import { IPythonExecutionService } from '../../../client/common/process/types';
 import { IDisposable } from '../../../client/common/types';
@@ -44,6 +45,7 @@ suite('Daemon', () => {
     let fullyQualifiedPythonPath: string = PYTHON_PATH;
     let pythonDaemon: PythonDaemonExecutionService;
     let pythonExecutionService: IPythonExecutionService;
+    let platformService: IPlatformService;
     let disposables: IDisposable[] = [];
     suiteSetup(() => {
         // When running locally.
@@ -67,8 +69,10 @@ suite('Daemon', () => {
         );
         connection.listen();
         pythonExecutionService = mock<IPythonExecutionService>();
+        platformService = mock<IPlatformService>();
         pythonDaemon = new PythonDaemonExecutionService(
             instance(pythonExecutionService),
+            instance(platformService),
             fullyQualifiedPythonPath,
             pythonProc,
             connection

src/test/common/process/pythonDaemonPool.functional.test.ts

@@ -15,6 +15,7 @@ import { Observable } from 'rxjs/Observable';
 import * as sinon from 'sinon';
 import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito';
 import { createMessageConnection, StreamMessageReader, StreamMessageWriter } from 'vscode-jsonrpc/node';
+import { IPlatformService } from '../../../client/common/platform/types';
 import { ProcessLogger } from '../../../client/common/process/logger';
 import { PythonDaemonExecutionServicePool } from '../../../client/common/process/pythonDaemonPool';
 import {
@@ -49,6 +50,7 @@ suite('Daemon - Python Daemon Pool', () => {
     let fullyQualifiedPythonPath: string = PYTHON_PATH;
     let pythonDaemonPool: PythonDaemonExecutionServicePool;
     let pythonExecutionService: IPythonExecutionService;
+    let platformService: IPlatformService;
     let disposables: IDisposable[] = [];
     let createDaemonServicesSpy: sinon.SinonSpy<[], Promise<IPythonDaemonExecutionService | IDisposable>>;
     let logger: IProcessLogger;
@@ -74,6 +76,7 @@ suite('Daemon - Python Daemon Pool', () => {
         logger = mock(ProcessLogger);
         createDaemonServicesSpy = sinon.spy(DaemonPool.prototype, 'createDaemonService');
         pythonExecutionService = mock<IPythonExecutionService>();
+        platformService = mock<IPlatformService>();
         when(
             pythonExecutionService.execModuleObservable('vscode_datascience_helpers.daemon', anything(), anything())
         ).thenCall(() => {
@@ -94,7 +97,15 @@ suite('Daemon - Python Daemon Pool', () => {
             daemonCount: 2,
             observableDaemonCount: 1
         };
-        pythonDaemonPool = new DaemonPool(logger, [], options, instance(pythonExecutionService), {}, 100);
+        pythonDaemonPool = new DaemonPool(
+            logger,
+            [],
+            options,
+            instance(pythonExecutionService),
+            instance(platformService),
+            {},
+            100
+        );
         await pythonDaemonPool.initialize();
         disposables.push(pythonDaemonPool);
     });

src/test/common/process/pythonDaemonPool.unit.test.ts

@@ -12,6 +12,7 @@ import { Observable } from 'rxjs/Observable';
 import * as sinon from 'sinon';
 import { anything, instance, mock, reset, verify, when } from 'ts-mockito';
 import { MessageConnection } from 'vscode-jsonrpc';
+import { IPlatformService } from '../../../client/common/platform/types';
 import { ProcessLogger } from '../../../client/common/process/logger';
 import { PythonDaemonExecutionService } from '../../../client/common/process/pythonDaemon';
 import { PythonDaemonExecutionServicePool } from '../../../client/common/process/pythonDaemonPool';
@@ -34,11 +35,13 @@ suite('Daemon - Python Daemon Pool', () => {
     // tslint:disable-next-line: no-any use-default-type-parameter
     let listenStub: sinon.SinonStub<any[], any>;
     let pythonExecService: IPythonExecutionService;
+    let platformService: IPlatformService;
     let logger: IProcessLogger;
     let clock: fakeTimers.InstalledClock;
     setup(() => {
         logger = instance(mock(ProcessLogger));
         pythonExecService = mock<IPythonExecutionService>();
+        platformService = mock<IPlatformService>();
         (instance(pythonExecService) as any).then = undefined;
         sendRequestStub = sinon.stub();
         listenStub = sinon.stub();
@@ -83,7 +86,14 @@ suite('Daemon - Python Daemon Pool', () => {
     }
     test('Create daemons when initializing', async () => {
         // Create and initialize the pool.
-        const pool = new DaemonPool(logger, [], { pythonPath: 'py.exe' }, instance(pythonExecService), undefined);
+        const pool = new DaemonPool(
+            logger,
+            [],
+            { pythonPath: 'py.exe' },
+            instance(pythonExecService),
+            instance(platformService),
+            undefined
+        );
         await setupDaemon(pool);
 
         // 2 = 2 for standard daemon + 1 observable daemon.
@@ -97,6 +107,7 @@ suite('Daemon - Python Daemon Pool', () => {
             [],
             { daemonCount: 5, observableDaemonCount: 3, pythonPath: 'py.exe' },
             instance(pythonExecService),
+            instance(platformService),
             undefined
         );
         await setupDaemon(pool);
@@ -115,6 +126,7 @@ suite('Daemon - Python Daemon Pool', () => {
             [],
             { daemonCount: 5, observableDaemonCount: 3, pythonPath: 'py.exe' },
             instance(pythonExecService),
+            instance(platformService),
             undefined
         );
         const promise = setupDaemon(pool);
@@ -143,6 +155,7 @@ suite('Daemon - Python Daemon Pool', () => {
             [],
             { daemonCount: 1, observableDaemonCount: 1, pythonPath: 'py.exe' },
             instance(pythonExecService),
+            instance(platformService),
             undefined
         );
         await setupDaemon(pool);
@@ -194,6 +207,7 @@ suite('Daemon - Python Daemon Pool', () => {
                 [],
                 { daemonCount: 2, observableDaemonCount: 1, pythonPath: 'py.exe' },
                 instance(pythonExecService),
+                instance(platformService),
                 undefined
             );
 
@@ -272,6 +286,7 @@ suite('Daemon - Python Daemon Pool', () => {
             [],
             { daemonCount: 1, observableDaemonCount: 1, pythonPath: 'py.exe' },
             instance(pythonExecService),
+            instance(platformService),
             undefined
         );
         await setupDaemon(pool);

src/test/common/process/pythonExecutionFactory.unit.test.ts

@@ -11,6 +11,7 @@ import { Uri } from 'vscode';
 
 import { PythonSettings } from '../../../client/common/configSettings';
 import { ConfigurationService } from '../../../client/common/configuration/service';
+import { IPlatformService } from '../../../client/common/platform/types';
 import { BufferDecoder } from '../../../client/common/process/decoder';
 import { ProcessLogger } from '../../../client/common/process/logger';
 import { ProcessServiceFactory } from '../../../client/common/process/processFactory';
@@ -87,13 +88,15 @@ suite('Process - PythonExecutionFactory', () => {
             let windowsStoreInterpreter: IWindowsStoreInterpreter;
             let interpreterService: IInterpreterService;
             let executionService: typemoq.IMock<IPythonExecutionService>;
+            let platformService: IPlatformService;
             setup(() => {
                 bufferDecoder = mock(BufferDecoder);
                 activationHelper = mock(EnvironmentActivationService);
                 processFactory = mock(ProcessServiceFactory);
                 configService = mock(ConfigurationService);
                 condaService = mock(CondaService);
                 processLogger = mock(ProcessLogger);
+                platformService = mock<IPlatformService>();
                 windowsStoreInterpreter = mock(WindowsStoreInterpreter);
                 executionService = typemoq.Mock.ofType<IPythonExecutionService>();
                 executionService.setup((p: any) => p.then).returns(() => undefined);
@@ -127,7 +130,8 @@ suite('Process - PythonExecutionFactory', () => {
                     instance(configService),
                     instance(condaService),
                     instance(bufferDecoder),
-                    instance(windowsStoreInterpreter)
+                    instance(windowsStoreInterpreter),
+                    instance(platformService)
                 );
             });
             teardown(() => sinon.restore());