Fix problems with liveshare (microsoft#4950)

- Host shutdown disconnects from jupyter - Host direct enter should send data to the guest even if guest doesn't have history window open For #4949 <!-- If an item below does not apply to you, then go ahead and check it off as "done" and strikethrough the text, e.g.: - [x] ~Has unit tests & system/integration tests~ --> - [x] Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR) - [x] Title summarizes what is changing - [x] Has a [news entry](https://github.com/Microsoft/vscode-python/tree/master/news) file (remember to thank yourself!) - [ ] Has sufficient logging. - [ ] Has telemetry for enhancements. - [x] Unit tests & system/integration tests are added/updated - [ ] [Test plan](https://github.com/Microsoft/vscode-python/blob/master/.github/test_plan.md) is updated as appropriate - [ ] [`package-lock.json`](https://github.com/Microsoft/vscode-python/blob/master/package-lock.json) has been regenerated by running `npm install` (if dependencies have changed) - [ ] The wiki is updated with any design decisions/details.

Author: rchiodo

news/2 Fixes/4949.md

@@ -0,0 +1 @@
+Shutting down a session should not cause the host to stop working.

src/client/common/application/types.ts

@@ -892,4 +892,5 @@ export interface ILiveShareTestingApi extends ILiveShareApi {
  isSessionStarted: boolean;
  forceRole(role: vsls.Role): void;
  startSession(): Promise<void>;
+ stopSession(): Promise<void>;
 }

src/client/datascience/history/history.ts

@@ -489,7 +489,13 @@ export class History implements IHistory {
  private submitNewCell(info: ISubmitNewCell) {
  // If there's any payload, it has the code and the id
  if (info && info.code && info.id) {
+ // Send to ourselves.
  this.submitCode(info.code, Identifiers.EmptyFileName, 0, info.id, undefined).ignoreErrors();
+
+ // Activate the other side, and send as if came from a file
+ this.historyProvider.getOrCreateActive().then(_v => {
+ this.shareMessage(HistoryMessages.RemoteAddCode, {code: info.code, file: Identifiers.EmptyFileName, line: 0, id: info.id, originator: this.id});
+ }).ignoreErrors();
  }
  }
 
@@ -667,7 +673,7 @@ export class History implements IHistory {
  if (this.jupyterServer) {
  const server = this.jupyterServer;
  this.jupyterServer = undefined;
- server.dispose().ignoreErrors(); // Don't care what happens as we're disconnected.
+ server.shutdown().ignoreErrors(); // Don't care what happens as we're disconnected.
  }
  }
  } catch {
@@ -839,6 +845,7 @@ export class History implements IHistory {
  }
 
  private shareMessage<M extends IHistoryMapping, T extends keyof M>(type: T, payload?: M[T]) {
+ // Send our remote message.
  this.messageListener.onMessage(type.toString(), payload);
  }
 

src/client/datascience/history/historyTypes.ts

@@ -35,7 +35,6 @@ export namespace HistoryMessages {
 // These are the messages that will mirror'd to guest/hosts in
 // a live share session
 export const HistoryRemoteMessages : string[] = [
- HistoryMessages.SubmitNewCell,
  HistoryMessages.AddedSysInfo,
  HistoryMessages.RemoteAddCode
 ];

src/client/datascience/jupyter/jupyterExecution.ts

@@ -11,6 +11,7 @@ import { CancellationToken, Event, EventEmitter } from 'vscode';
 
 import { ILiveShareApi, IWorkspaceService } from '../../common/application/types';
 import { Cancellation, CancellationError } from '../../common/cancellation';
+import { traceInfo, traceWarning } from '../../common/logger';
 import { IFileSystem, TemporaryDirectory } from '../../common/platform/types';
 import { IProcessService, IProcessServiceFactory, IPythonExecutionFactory, SpawnOptions } from '../../common/process/types';
 import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry, ILogger } from '../../common/types';
@@ -34,7 +35,6 @@ import {
 } from '../types';
 import { JupyterConnection, JupyterServerInfo } from './jupyterConnection';
 import { JupyterKernelSpec } from './jupyterKernelSpec';
-import { traceInfo, traceWarning } from '../../common/logger';
 
 enum ModuleExistsResult {
  NotFound,
@@ -698,6 +698,7 @@ export class JupyterExecutionBase implements IJupyterExecution {
  // - Look for module in current interpreter, if found create something with python path and -m module
  // - Look in other interpreters, if found create something with python path and -m module
  // - Look on path for jupyter, if found create something with jupyter path and args
+ // tslint:disable:cyclomatic-complexity
  private findBestCommand = async (command: string, cancelToken?: CancellationToken): Promise<IJupyterCommand | undefined> => {
  // See if we already have this command in list
  if (!this.commands.hasOwnProperty(command)) {
@@ -714,7 +715,7 @@ export class JupyterExecutionBase implements IJupyterExecution {
  const all = await this.interpreterService.getInterpreters();
 
  if (!all || all.length === 0) {
- traceWarning(`No interpreters found. Jupyter cannot run.`);
+ traceWarning('No interpreters found. Jupyter cannot run.');
  }
 
  const promises = all.filter(i => i !== current).map(i => this.findInterpreterCommand(command, i, cancelToken));

src/client/datascience/jupyter/liveshare/hostJupyterExecution.ts

@@ -109,8 +109,12 @@ export class HostJupyterExecution
  public async onDetach(api: vsls.LiveShare | null): Promise<void> {
  await super.onDetach(api);
 
- // clear our cached servers. We need to reconnect
- await this.serverCache.dispose();
+ // clear our cached servers if our role is no longer host or none
+ const newRole = api === null || (api.session && api.session.role !== vsls.Role.Guest) ?
+ vsls.Role.Host : vsls.Role.Guest;
+ if (newRole !== vsls.Role.Host) {
+ await this.serverCache.dispose();
+ }
  }
 
  public getServer(options?: INotebookServerOptions): Promise<INotebookServer | undefined> {

src/test/datascience/liveshare.functional.test.tsx

@@ -259,6 +259,12 @@ suite('LiveShare tests', () => {
  return api.startSession();
  }
 
+ function stopSession(role: vsls.Role): Promise<void> {
+ const container = role === vsls.Role.Host ? hostContainer : guestContainer;
+ const api = container.ioc!.get<ILiveShareApi>(ILiveShareApi) as ILiveShareTestingApi;
+ return api.stopSession();
+ }
+
  test('Host alone', async () => {
  // Should only need mock data in host
  addMockData(hostContainer.ioc!, 'a=1\na', 1);
@@ -290,6 +296,26 @@ suite('LiveShare tests', () => {
  verifyHtmlOnCell(guestContainer.wrapper!, '<span>1</span>', CellPosition.Last);
  });
 
+ test('Host Shutdown and Run', async () => {
+ // Should only need mock data in host
+ addMockData(hostContainer.ioc!, 'a=1\na', 1);
+
+ // Create the host history and then the guest history
+ await getOrCreateHistory(vsls.Role.Host);
+ await startSession(vsls.Role.Host);
+
+ // Send code through the host
+ let wrapper = await addCodeToRole(vsls.Role.Host, 'a=1\na');
+ verifyHtmlOnCell(wrapper, '<span>1</span>', CellPosition.Last);
+
+ // Stop the session
+ await stopSession(vsls.Role.Host);
+
+ // Send code again. It should still work.
+ wrapper = await addCodeToRole(vsls.Role.Host, 'a=1\na');
+ verifyHtmlOnCell(wrapper, '<span>1</span>', CellPosition.Last);
+ });
+
  test('Host startup and guest restart', async () => {
  // Should only need mock data in host
  addMockData(hostContainer.ioc!, 'a=1\na', 1);

src/test/datascience/mockLiveShare.ts

@@ -195,6 +195,14 @@ class MockLiveShare implements vsls.LiveShare, vsls.Session, vsls.Peer {
  return Promise.resolve();
  }
 
+ public async stop(): Promise<void> {
+ this._visibleRole = vsls.Role.None;
+
+ await this.changeSessionEmitter.fire({ session: this });
+
+ return Promise.resolve();
+ }
+
  public getContacts(_emails: string[]): Promise<vsls.ContactsCollection> {
  throw new Error('Method not implemented.');
  }
@@ -337,6 +345,15 @@ export class MockLiveShareApi implements ILiveShareTestingApi {
  }
  }
 
+ public async stopSession(): Promise<void> {
+ if (this.currentApi) {
+ await this.currentApi.stop();
+ this.sessionStarted = false;
+ } else {
+ throw Error('Cannot start session without a role.');
+ }
+ }
+
  public get isSessionStarted(): boolean {
  return this.sessionStarted;
  }