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

Dispose of file watchers when web TS server exits #185815

Merged
merged 1 commit into from
Jun 22, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export class DiagnosticsManager extends Disposable {
public override dispose() {
super.dispose();

for (const value of this._pendingUpdates.values) {
for (const value of this._pendingUpdates.values()) {
clearTimeout(value);
}
this._pendingUpdates.clear();
Expand Down Expand Up @@ -259,7 +259,7 @@ export class DiagnosticsManager extends Disposable {

private rebuildAll(): void {
this._currentDiagnostics.clear();
for (const fileDiagnostic of this._diagnostics.values) {
for (const fileDiagnostic of this._diagnostics.values()) {
this.rebuildFile(fileDiagnostic);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class BufferSynchronizer {
const closedFiles: string[] = [];
const openFiles: Proto.OpenRequestArgs[] = [];
const changedFiles: Proto.FileCodeEdits[] = [];
for (const change of this._pending.values) {
for (const change of this._pending.values()) {
switch (change.type) {
case BufferOperationType.Change: changedFiles.push(change.args); break;
case BufferOperationType.Open: openFiles.push(change.args); break;
Expand Down Expand Up @@ -270,13 +270,13 @@ class SyncedBufferMap extends ResourceMap<SyncedBuffer> {
}

public get allBuffers(): Iterable<SyncedBuffer> {
return this.values;
return this.values();
}
}

class PendingDiagnostics extends ResourceMap<number> {
public getOrderedFileSet(): ResourceMap<void> {
const orderedResources = Array.from(this.entries)
const orderedResources = Array.from(this.entries())
.sort((a, b) => a.value - b.value)
.map(entry => entry.resource);

Expand Down Expand Up @@ -313,7 +313,7 @@ class GetErrRequest {
}

const supportsSyntaxGetErr = this.client.apiVersion.gte(API.v440);
const allFiles = coalesce(Array.from(files.entries)
const allFiles = coalesce(Array.from(files.entries())
.filter(entry => supportsSyntaxGetErr || client.hasCapabilityForResource(entry.resource, ClientCapability.Semantic))
.map(entry => client.toTsFilePath(entry.resource)));

Expand Down Expand Up @@ -711,7 +711,7 @@ export default class BufferSyncSupport extends Disposable {
if (this.pendingGetErr) {
this.pendingGetErr.cancel();

for (const { resource } of this.pendingGetErr.files.entries) {
for (const { resource } of this.pendingGetErr.files.entries()) {
if (this.syncedBuffers.get(resource)) {
orderedFileSet.set(resource, undefined);
}
Expand All @@ -721,7 +721,7 @@ export default class BufferSyncSupport extends Disposable {
}

// Add all open TS buffers to the geterr request. They might be visible
for (const buffer of this.syncedBuffers.values) {
for (const buffer of this.syncedBuffers.values()) {
orderedFileSet.set(buffer.resource, undefined);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import { Logger } from '../logging/logger';
import { disposeAll, IDisposable } from '../utils/dispose';
import { ResourceMap } from '../utils/resourceMap';

type DirWatcherEntry = {
interface DirWatcherEntry {
readonly uri: vscode.Uri;
readonly listeners: IDisposable[];
};
}


export class FileWatcherManager {
export class FileWatcherManager implements IDisposable {

private readonly _fileWatchers = new Map<number, {
readonly uri: vscode.Uri;
Expand All @@ -34,6 +34,18 @@ export class FileWatcherManager {
private readonly logger: Logger,
) { }

dispose(): void {
for (const entry of this._fileWatchers.values()) {
entry.watcher.dispose();
}
this._fileWatchers.clear();

for (const entry of this._dirWatchers.values()) {
entry.watcher.dispose();
}
this._dirWatchers.clear();
}

create(id: number, uri: vscode.Uri, watchParentDirs: boolean, isRecursive: boolean, listeners: { create?: (uri: vscode.Uri) => void; change?: (uri: vscode.Uri) => void; delete?: (uri: vscode.Uri) => void }): void {
this.logger.trace(`Creating file watcher for ${uri.toString()}`);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,16 @@ class WorkerServerProcess implements TsServerProcess {
private readonly _onDataHandlers = new Set<(data: Proto.Response) => void>();
private readonly _onErrorHandlers = new Set<(err: Error) => void>();
private readonly _onExitHandlers = new Set<(code: number | null, signal: string | null) => void>();
private readonly _watches: FileWatcherManager;

private readonly worker: Worker;
private readonly _worker: Worker;
private readonly _watches: FileWatcherManager;

/** For communicating with TS server synchronously */
private readonly tsserver: MessagePort;
private readonly _tsserver: MessagePort;
/** For communicating watches asynchronously */
private readonly watcher: MessagePort;
private readonly _watcher: MessagePort;
/** For communicating with filesystem synchronously */
private readonly syncFs: MessagePort;
private readonly _syncFs: MessagePort;

public constructor(
private readonly kind: TsServerProcessKind,
Expand All @@ -81,18 +81,18 @@ class WorkerServerProcess implements TsServerProcess {
private readonly tsServerLog: TsServerLog | undefined,
logger: Logger,
) {
this.worker = new Worker(tsServerPath, { name: `TS ${kind} server #${this.id}` });
this._worker = new Worker(tsServerPath, { name: `TS ${kind} server #${this.id}` });

this._watches = new FileWatcherManager(logger);

const tsserverChannel = new MessageChannel();
const watcherChannel = new MessageChannel();
const syncChannel = new MessageChannel();
this.tsserver = tsserverChannel.port2;
this.watcher = watcherChannel.port2;
this.syncFs = syncChannel.port2;
this._tsserver = tsserverChannel.port2;
this._watcher = watcherChannel.port2;
this._syncFs = syncChannel.port2;

this.tsserver.onmessage = (event) => {
this._tsserver.onmessage = (event) => {
if (event.data.type === 'log') {
console.error(`unexpected log message on tsserver channel: ${JSON.stringify(event)}`);
return;
Expand All @@ -102,7 +102,7 @@ class WorkerServerProcess implements TsServerProcess {
}
};

this.watcher.onmessage = (event: MessageEvent<BrowserWatchEvent>) => {
this._watcher.onmessage = (event: MessageEvent<BrowserWatchEvent>) => {
switch (event.data.type) {
case 'dispose': {
this._watches.delete(event.data.id);
Expand All @@ -111,9 +111,9 @@ class WorkerServerProcess implements TsServerProcess {
case 'watchDirectory':
case 'watchFile': {
this._watches.create(event.data.id, vscode.Uri.from(event.data.uri), /*watchParentDirs*/ true, !!event.data.recursive, {
change: uri => this.watcher.postMessage({ type: 'watch', event: 'change', uri }),
create: uri => this.watcher.postMessage({ type: 'watch', event: 'create', uri }),
delete: uri => this.watcher.postMessage({ type: 'watch', event: 'delete', uri }),
change: uri => this._watcher.postMessage({ type: 'watch', event: 'change', uri }),
create: uri => this._watcher.postMessage({ type: 'watch', event: 'create', uri }),
delete: uri => this._watcher.postMessage({ type: 'watch', event: 'delete', uri }),
});
break;
}
Expand All @@ -122,7 +122,7 @@ class WorkerServerProcess implements TsServerProcess {
}
};

this.worker.onmessage = (msg: any) => {
this._worker.onmessage = (msg: any) => {
// for logging only
if (msg.data.type === 'log') {
this.appendLog(msg.data.body);
Expand All @@ -131,15 +131,15 @@ class WorkerServerProcess implements TsServerProcess {
console.error(`unexpected message on main channel: ${JSON.stringify(msg)}`);
};

this.worker.onerror = (err: ErrorEvent) => {
this._worker.onerror = (err: ErrorEvent) => {
console.error('error! ' + JSON.stringify(err));
for (const handler of this._onErrorHandlers) {
// TODO: The ErrorEvent type might be wrong; previously this was typed as Error and didn't have the property access.
handler(err.error);
}
};

this.worker.postMessage(
this._worker.postMessage(
{ args, extensionUri },
[syncChannel.port1, tsserverChannel.port1, watcherChannel.port1]
);
Expand All @@ -150,7 +150,7 @@ class WorkerServerProcess implements TsServerProcess {
}

write(serverRequest: Proto.Request): void {
this.tsserver.postMessage(serverRequest);
this._tsserver.postMessage(serverRequest);
}

onData(handler: (response: Proto.Response) => void): void {
Expand All @@ -167,10 +167,11 @@ class WorkerServerProcess implements TsServerProcess {
}

kill(): void {
this.worker.terminate();
this.tsserver.close();
this.watcher.close();
this.syncFs.close();
this._worker.terminate();
this._tsserver.close();
this._watcher.close();
this._syncFs.close();
this._watches.dispose();
}

private appendLog(msg: string) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,11 @@ export class ResourceMap<T> {
this._map.clear();
}

public get values(): Iterable<T> {
public values(): Iterable<T> {
return Array.from(this._map.values(), x => x.value);
}

public get entries(): Iterable<{ resource: vscode.Uri; value: T }> {
public entries(): Iterable<{ resource: vscode.Uri; value: T }> {
return this._map.values();
}

Expand Down