Fix linux tests to report correctly, get rid of stream destroyed messages on raw kernel shutdown (microsoft#12643)

Author: IanMatthewHuff

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);