-
Notifications
You must be signed in to change notification settings - Fork 324
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
Add fifo
as a transport option
#1351
Comments
Unix Domain Socket (UDS) causes a problem in python for adaption of pip transport in LSP/DAP/testing where we need to communicate back to node process and we use VSCode JSON RPC package. When I say LSP/DAP/testing, we have plans to migrate all socket communication to use namedpipes. There are packages in python that explicitly disable sockets on load, and we often get bitten by firewall rules. Stdio communication also gets impacted by packages trying to write to stdio on load. Sometimes this occurs even before any of our packages are loaded for us to capture the stdio file descriptors. Another way this is a problem is when, pythons logging is enabled to write logs to stdout. We have several reasons to get off of stdio, and sockets is not a good option, so pipes are the next best thing. How we discovered this problem: in python named pipes can be connected to using The recommended change is on non-windows to use |
@karthiknadig could you please point me to the |
@dbaeumer I will share code sample. You basically spawn |
Here is an example: let connectResolve: (value: [rpc.MessageReader, rpc.MessageWriter]) => void;
const connected = new Promise<[rpc.MessageReader, rpc.MessageWriter]>((resolve, _reject) => {
connectResolve = resolve;
});
return new Promise((resolve, reject) => {
const proc = child_process.spawn('mkfifo', ['-m', '0666', pipeName]);
proc.on('error', reject);
proc.on('exit', (code, signal) => {
if (code !== 0) {
reject(new Error(`Failed to create fifo pipe ${pipeName}: ${signal}`));
}
fs.open(pipeName, constants.O_RDWR, (err, fd) => {
if (err) {
reject(err);
}
const socket = new net.Socket({ fd, readable: true, writable: true });
connectResolve([
new rpc.SocketMessageReader(socket, 'utf-8'),
new rpc.SocketMessageWriter(socket, 'utf-8'),
]);
});
resolve({ onConnected: () => connected });
});
}); You can also replace |
@karthiknadig thanks for the code snippet. Any desire to provide a PR for this :-) This code should go somewhere here:
I still think that we need to add a new transport kind |
@dbaeumer I investigated more into how So, I don't think we can switch the communication over from stdio for python scenarios unfortunately. |
@karthiknadig thanks for the investigation. One idea for working around the ordering is to have some ready/tag files in the file system to synchronize this. |
fifo
not Unix Domain Socketfifo
as a transport option
This code should use
fs.mkfifo
on non-windows:vscode-languageserver-node/jsonrpc/src/node/main.ts
Lines 201 to 220 in 277e456
The text was updated successfully, but these errors were encountered: