Skip to content

Commit

Permalink
server: fix socket pipe disconnections not getting detected (#214640)
Browse files Browse the repository at this point in the history
Fixes #211462
  • Loading branch information
connor4312 authored Jun 7, 2024
1 parent b814ff2 commit dc97013
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 2 deletions.
14 changes: 14 additions & 0 deletions src/vs/base/parts/ipc/node/ipc.net.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ import { generateUuid } from 'vs/base/common/uuid';
import { ClientConnectionEvent, IPCServer } from 'vs/base/parts/ipc/common/ipc';
import { ChunkStream, Client, ISocket, Protocol, SocketCloseEvent, SocketCloseEventType, SocketDiagnostics, SocketDiagnosticsEventType } from 'vs/base/parts/ipc/common/ipc.net';

/**
* Maximum time to wait for a 'close' event to fire after the socket stream
* ends. For unix domain sockets, the close event may not fire consistently
* due to what appears to be a Node.js bug.
*
* @see https://github.com/microsoft/vscode/issues/211462#issuecomment-2155471996
*/
const socketEndTimeoutMs = 30_000;

export class NodeSocket implements ISocket {

public readonly debugLabel: string;
Expand Down Expand Up @@ -51,15 +60,20 @@ export class NodeSocket implements ISocket {
};
this.socket.on('error', this._errorListener);

let endTimeoutHandle: NodeJS.Timeout | undefined;
this._closeListener = (hadError: boolean) => {
this.traceSocketEvent(SocketDiagnosticsEventType.Close, { hadError });
this._canWrite = false;
if (endTimeoutHandle) {
clearTimeout(endTimeoutHandle);
}
};
this.socket.on('close', this._closeListener);

this._endListener = () => {
this.traceSocketEvent(SocketDiagnosticsEventType.NodeEndReceived);
this._canWrite = false;
endTimeoutHandle = setTimeout(() => socket.destroy(), socketEndTimeoutMs);
};
this.socket.on('end', this._endListener);
}
Expand Down
25 changes: 23 additions & 2 deletions src/vs/base/parts/ipc/test/node/ipc.net.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
*--------------------------------------------------------------------------------------------*/

import assert from 'assert';
import sinon from 'sinon';
import { EventEmitter } from 'events';
import { AddressInfo, connect, createServer, Server, Socket } from 'net';
import { tmpdir } from 'os';
import { Barrier, timeout } from 'vs/base/common/async';
import { VSBuffer } from 'vs/base/common/buffer';
import { Emitter, Event } from 'vs/base/common/event';
import { Disposable, DisposableStore } from 'vs/base/common/lifecycle';
import { Disposable, DisposableStore, toDisposable } from 'vs/base/common/lifecycle';
import { ILoadEstimator, PersistentProtocol, Protocol, ProtocolConstants, SocketCloseEvent, SocketDiagnosticsEventType } from 'vs/base/parts/ipc/common/ipc.net';
import { createRandomIPCHandle, createStaticIPCHandle, NodeSocket, WebSocketNodeSocket } from 'vs/base/parts/ipc/node/ipc.net';
import { flakySuite } from 'vs/base/test/common/testUtils';
Expand Down Expand Up @@ -134,7 +135,7 @@ class Ether {

suite('IPC, Socket Protocol', () => {

ensureNoDisposablesAreLeakedInTestSuite();
const ds = ensureNoDisposablesAreLeakedInTestSuite();

let ether: Ether;

Expand Down Expand Up @@ -186,6 +187,26 @@ suite('IPC, Socket Protocol', () => {
b.dispose();
});



test('issue #211462: destroy socket after end timeout', async () => {
const socket = new EventEmitter();
Object.assign(socket, { destroy: () => socket.emit('close') });
const protocol = ds.add(new Protocol(new NodeSocket(socket as Socket)));

const disposed = sinon.stub();
const timers = sinon.useFakeTimers();

ds.add(toDisposable(() => timers.restore()));
ds.add(protocol.onDidDispose(disposed));

socket.emit('end');
assert.ok(!disposed.called);
timers.tick(29_999);
assert.ok(!disposed.called);
timers.tick(1);
assert.ok(disposed.called);
});
});

suite('PersistentProtocol reconnection', () => {
Expand Down

0 comments on commit dc97013

Please sign in to comment.