microsoft / vscode-languageserver-node

Language server protocol implementation for VSCode. This allows implementing language services in JS/TS running on node.js
MIT License
1.47k stars 324 forks source link

Add `fifo` as a transport option #1351

Open karthiknadig opened 1 year ago

karthiknadig commented 1 year ago

This code should use fs.mkfifo on non-windows: https://github.com/microsoft/vscode-languageserver-node/blob/277e4564193c4ed258fe4d8d405d3379665bbab9/jsonrpc/src/node/main.ts#L201-L220

karthiknadig commented 1 year ago

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 open, like with open("<pipe-name", "r+b") as pipe: gives you a stream object that read/write to pipes. This should work on all os'es. It worked on windows, when I tested this on linux, open would come back with file does not exist. I could connect to the node "pipe" using netcat or by using python sockets by setting the socket family to (AF_UNIX). Both of which means that it is using Unix Domain Socket.

The recommended change is on non-windows to use mkfifo to create the namedpipe (or fifo), and provide a similar wrapper to expose the read write streams.

dbaeumer commented 1 year ago

@karthiknadig could you please point me to the fs.mkfifo in NodeJS. I was not able to find it. All I could find is stat related API to detect if a handle is a fifo.

karthiknadig commented 1 year ago

@dbaeumer I will share code sample. You basically spawn mkfifo as a subprocess to create it. (https://www.gnu.org/software/coreutils/mkfifo)

karthiknadig commented 1 year ago

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 net.Socket with just plain stream from fs.createReadStream and fs.createWriteStream.

dbaeumer commented 1 year ago

@karthiknadig thanks for the code snippet. Any desire to provide a PR for this :-) This code should go somewhere here: https://github.com/microsoft/vscode-languageserver-node/blob/f8a54a2b35e9a2781c711a145be8acc44ca6902f/jsonrpc/src/node/main.ts#L201 and https://github.com/microsoft/vscode-languageserver-node/blob/f8a54a2b35e9a2781c711a145be8acc44ca6902f/jsonrpc/src/node/main.ts#L224

I still think that we need to add a new transport kind fifo for this. Otherwise we might break things if users upgrade the client but not the server.

karthiknadig commented 1 year ago

@dbaeumer I investigated more into how fifo are done in various linux variants and mac. There is lot of variance in how fifo are handled in each OS. The main issues comes down to read-write ordering. The requirement is that the read side should open the fifo in read mode, before the write side opens it. LS Client should open the fifo in read mode and wait for LS Server to open the fifo in read mode on its side before both LS Client and LS Server open write mode streams. This coordination is going to be difficult, as we don't really have a protocol way to say server has connected. fifos don't have a way to detect this, we will need to do this manually. The behavior of this ordering varies between flavors on linux. The fact that there is an ordering requirement is a blocker to me.

So, I don't think we can switch the communication over from stdio for python scenarios unfortunately.

dbaeumer commented 1 year ago

@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.