Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to named pipes #1327

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/debugAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import fs = require("fs");
import net = require("net");
import os = require("os");
import path = require("path");
import { Logger } from "./logging";
import utils = require("./utils");
Expand Down Expand Up @@ -39,10 +40,10 @@ function startDebugging() {
utils.deleteSessionFile(debugSessionFilePath);

// Establish connection before setting up the session
debugAdapterLogWriter.write("Connecting to port: " + sessionDetails.debugServicePort + "\r\n");
debugAdapterLogWriter.write("Connecting to pipe: " + sessionDetails.debugServicePipeName + "\r\n");

let isConnected = false;
const debugServiceSocket = net.connect(sessionDetails.debugServicePort, "127.0.0.1");
const debugServiceSocket = net.connect(utils.getPipePath(sessionDetails.debugServicePipeName));

// Write any errors to the log file
debugServiceSocket.on(
Expand Down Expand Up @@ -73,7 +74,7 @@ function startDebugging() {

// Resume the stdin stream
process.stdin.resume();
});
});

// When the socket closes, end the session
debugServiceSocket.on(
Expand Down
25 changes: 15 additions & 10 deletions src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*--------------------------------------------------------*/

import cp = require("child_process");
import crypto = require("crypto");
import fs = require("fs");
import net = require("net");
import os = require("os");
Expand Down Expand Up @@ -165,19 +166,23 @@ export class SessionManager implements Middleware {
}
}

// Generate a random id for the named pipes in case they have multiple instances of PSES running
const id = crypto.randomBytes(10).toString("hex");
this.editorServicesArgs =
"-HostName 'Visual Studio Code Host' " +
"-HostProfileId 'Microsoft.VSCode' " +
"-HostVersion '" + this.hostVersion + "' " +
"-AdditionalModules @('PowerShellEditorServices.VSCode') " +
"-BundledModulesPath '" + this.bundledModulesPath + "' " +
"-EnableConsoleRepl ";
`-HostName 'Visual Studio Code Host' ` +
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on using ordinary strings where no interpolation is needed vs consistency? Not opinionated either way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for here, I just kept it consistent with the rest of the string concat

`-HostProfileId 'Microsoft.VSCode' ` +
`-HostVersion '${this.hostVersion}'` +
`-AdditionalModules @('PowerShellEditorServices.VSCode') ` +
`-BundledModulesPath '${this.bundledModulesPath}'` +
`-EnableConsoleRepl ` +
`-LanguageServicePipeName LanguageService_${id}.pipe ` +
`-DebugServicePipeName DebugService_${id}.pipe `;

if (this.sessionSettings.developer.editorServicesWaitForDebugger) {
this.editorServicesArgs += "-WaitForDebugger ";
}
if (this.sessionSettings.developer.editorServicesLogLevel) {
this.editorServicesArgs += "-LogLevel '" + this.sessionSettings.developer.editorServicesLogLevel + "' ";
this.editorServicesArgs += `-LogLevel '${this.sessionSettings.developer.editorServicesLogLevel}' `;
}

this.startPowerShell();
Expand Down Expand Up @@ -531,18 +536,18 @@ export class SessionManager implements Middleware {

private startLanguageClient(sessionDetails: utils.IEditorServicesSessionDetails) {

const port = sessionDetails.languageServicePort;
const pipeName = sessionDetails.languageServicePipeName;

// Log the session details object
this.log.write(JSON.stringify(sessionDetails));

try {
this.log.write("Connecting to language service on port " + port + "...");
this.log.write("Connecting to language service on pipe " + pipeName + "...");

const connectFunc = () => {
return new Promise<StreamInfo>(
(resolve, reject) => {
const socket = net.connect(port);
const socket = net.connect(utils.getPipePath(pipeName));
socket.on(
"connect",
() => {
Expand Down
14 changes: 5 additions & 9 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,9 @@ export function getPipePath(pipeName: string) {
if (os.platform() === "win32") {
return "\\\\.\\pipe\\" + pipeName;
} else {
// On UNIX platforms the pipe will live under the temp path
// For details on how this path is computed, see the corefx
// source for System.IO.Pipes.PipeStream:
// tslint:disable-next-line:max-line-length
// https://github.com/dotnet/corefx/blob/d0dc5fc099946adc1035b34a8b1f6042eddb0c75/src/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs#L340
return path.resolve(
os.tmpdir(),
".dotnet", "corefx", "pipe",
pipeName);
// Windows uses NamedPipes where non-Windows platforms use Unix Domain Sockets.
// This requires connecting to the pipe file in different locations on Windows vs non-Windows.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically the name difference is just .NET Core being strange, rather than the use of domain sockets, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nah the location is different too. Named Pipes are in:

\\.\pipe\foo

domain sockets are in:

/tmp/CoreFxPipe_foo

Even the Named Pipes on Windows using .NET Standard instead are placed here:

\\.\pipe\foo

return path.join(os.tmpdir(), `CoreFxPipe_${pipeName}`);
}
}

Expand All @@ -46,6 +40,8 @@ export interface IEditorServicesSessionDetails {
channel: string;
languageServicePort: number;
debugServicePort: number;
languageServicePipeName: string;
debugServicePipeName: string;
}

export type IReadSessionFileCallback = (details: IEditorServicesSessionDetails) => void;
Expand Down