Fix crash when trying to forward two ports to the same port

alexr00 · alexr00 · commit a949bf8de44c · 2020-01-28T16:53:17.000+01:00
Also change error to warn so that it doesn't show in the console Fixes microsoft#89419
diff --git a/src/vs/workbench/contrib/remote/browser/tunnelView.ts b/src/vs/workbench/contrib/remote/browser/tunnelView.ts
@@ -680,7 +680,7 @@ namespace ForwardPortAction {
 
 	function error(notificationService: INotificationService, tunnel: RemoteTunnel | void, host: string, port: number) {
 		if (!tunnel) {
-			notificationService.error(nls.localize('remote.tunnel.forwardError', "Unable to forward {0}:{1}. The host may not be available.", host, port));
+			notificationService.warn(nls.localize('remote.tunnel.forwardError', "Unable to forward {0}:{1}. The host may not be available or that remote port may already be forwarded", host, port));
 		}
 	}
 
@@ -865,14 +865,19 @@ namespace ChangeLocalPortAction {
 	export function handler(): ICommandHandler {
 		return async (accessor, arg) => {
 			const remoteExplorerService = accessor.get(IRemoteExplorerService);
+			const notificationService = accessor.get(INotificationService);
 			const context = (arg !== undefined || arg instanceof TunnelItem) ? arg : accessor.get(IContextKeyService).getContextKeyValue(TunnelViewSelectionKeyName);
 			if (context instanceof TunnelItem) {
 				remoteExplorerService.setEditable(context, {
 					onFinish: async (value, success) => {
 						remoteExplorerService.setEditable(context, null);
 						if (success) {
 							await remoteExplorerService.close({ host: context.remoteHost, port: context.remotePort });
-							await remoteExplorerService.forward({ host: context.remoteHost, port: context.remotePort }, Number(value));
+							const numberValue = Number(value);
+							const newForward = await remoteExplorerService.forward({ host: context.remoteHost, port: context.remotePort }, numberValue);
+							if (newForward && newForward.tunnelLocalPort !== numberValue) {
+								notificationService.warn(nls.localize('remote.tunnel.changeLocalPortNumber', "The local port {0} is not available. Port number {1} has been used instead", value, newForward.tunnelLocalPort));
+							}
 						}
 					},
 					validationMessage: validateInput,
diff --git a/src/vs/workbench/services/remote/node/tunnelService.ts b/src/vs/workbench/services/remote/node/tunnelService.ts
@@ -37,6 +37,7 @@ class NodeRemoteTunnel extends Disposable implements RemoteTunnel {
 
 	private readonly _listeningListener: () => void;
 	private readonly _connectionListener: (socket: net.Socket) => void;
+	private readonly _errorListener: () => void;
 
 	constructor(options: IConnectionOptions, tunnelRemoteHost: string, tunnelRemotePort: number, private readonly suggestedLocalPort?: number) {
 		super();
@@ -50,6 +51,10 @@ class NodeRemoteTunnel extends Disposable implements RemoteTunnel {
 		this._connectionListener = (socket) => this._onConnection(socket);
 		this._server.on('connection', this._connectionListener);
 
+		// If there is no error listener and there is an error it will crash the whole window
+		this._errorListener = () => { };
+		this._server.on('error', this._errorListener);
+
 		this.tunnelRemotePort = tunnelRemotePort;
 		this.tunnelRemoteHost = tunnelRemoteHost;
 	}
@@ -58,16 +63,24 @@ class NodeRemoteTunnel extends Disposable implements RemoteTunnel {
 		super.dispose();
 		this._server.removeListener('listening', this._listeningListener);
 		this._server.removeListener('connection', this._connectionListener);
+		this._server.removeListener('error', this._errorListener);
 		this._server.close();
 	}
 
 	public async waitForReady(): Promise<this> {
-
 		// try to get the same port number as the remote port number...
-		const localPort = await findFreePortFaster(this.suggestedLocalPort ?? this.tunnelRemotePort, 2, 1000);
+		let localPort = await findFreePortFaster(this.suggestedLocalPort ?? this.tunnelRemotePort, 2, 1000);
 
 		// if that fails, the method above returns 0, which works out fine below...
-		const address = (<net.AddressInfo>this._server.listen(localPort).address());
+		let address: string | net.AddressInfo | null = null;
+		address = (<net.AddressInfo>this._server.listen(localPort).address());
+
+		// It is possible for findFreePortFaster to return a port that there is already a server listening on. This causes the previous listen call to error out.
+		if (!address) {
+			localPort = 0;
+			address = (<net.AddressInfo>this._server.listen(localPort).address());
+		}
+
 		this.tunnelLocalPort = address.port;
 
 		await this._barrier.wait();
