Error: disposed around DiskFileSystemProvider.doRefreshRecursiveWatchers (fix #135530)

src/vs/platform/files/common/diskFileSystemProvider.ts

@@ -69,22 +69,17 @@ export abstract class AbstractDiskFileSystemProvider extends Disposable {
 
 		// Buffer requests for recursive watching to decide on right watcher
 		// that supports potentially watching more than one folder at once
-		this.recursiveWatchRequestDelayer.trigger(async () => {
-			this.doRefreshRecursiveWatchers();
-		});
+		this.recursiveWatchRequestDelayer.trigger(() => {
+			return this.doRefreshRecursiveWatchers();
+		}).catch(error => this.logService.error(error));
 	}
 
-	private doRefreshRecursiveWatchers(): void {
+	private doRefreshRecursiveWatchers(): Promise<void> {
 
-		// Reuse existing
-		if (this.recursiveWatcher) {
-			this.recursiveWatcher.watch(this.recursiveFoldersToWatch);
-		}
-
-		// Otherwise, create new if we have folders to watch
-		else if (this.recursiveFoldersToWatch.length > 0) {
+		// Create watcher if this is the first time
+		if (!this.recursiveWatcher) {
 			this.recursiveWatcher = this._register(this.createRecursiveWatcher(
-				this.recursiveFoldersToWatch,
+				this.recursiveFoldersToWatch.length,
 				changes => this._onDidChangeFile.fire(toFileChanges(changes)),
 				msg => this.onWatcherLogMessage(msg),
 				this.logService.getLevel() === LogLevel.Trace
@@ -95,10 +90,13 @@ export abstract class AbstractDiskFileSystemProvider extends Disposable {
 				this.recursiveWatcher?.setVerboseLogging(this.logService.getLevel() === LogLevel.Trace);
 			}));
 		}
+
+		// Ask to watch the provided folders
+		return this.recursiveWatcher.watch(this.recursiveFoldersToWatch);
 	}
 
 	protected abstract createRecursiveWatcher(
-		folders: IWatchRequest[],
+		folders: number,
 		onChange: (changes: IDiskFileChange[]) => void,
 		onLogMessage: (msg: ILogMessage) => void,
 		verboseLogging: boolean

src/vs/platform/files/node/diskFileSystemProvider.ts

@@ -24,7 +24,7 @@ import { FileWatcher as NodeJSWatcherService } from 'vs/platform/files/node/watc
 import { FileWatcher as NsfwWatcherService } from 'vs/platform/files/node/watcher/nsfw/watcherService';
 import { FileWatcher as ParcelWatcherService } from 'vs/platform/files/node/watcher/parcel/watcherService';
 import { FileWatcher as UnixWatcherService } from 'vs/platform/files/node/watcher/unix/watcherService';
-import { IDiskFileChange, ILogMessage, IWatchRequest, WatcherService } from 'vs/platform/files/common/watcher';
+import { IDiskFileChange, ILogMessage, WatcherService } from 'vs/platform/files/common/watcher';
 import { ILogService } from 'vs/platform/log/common/log';
 import product from 'vs/platform/product/common/product';
 import { AbstractDiskFileSystemProvider } from 'vs/platform/files/common/diskFileSystemProvider';
@@ -538,14 +538,13 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple
 	//#region File Watching
 
 	protected createRecursiveWatcher(
-		folders: IWatchRequest[],
+		folders: number,
 		onChange: (changes: IDiskFileChange[]) => void,
 		onLogMessage: (msg: ILogMessage) => void,
 		verboseLogging: boolean
 	): WatcherService {
 		let watcherImpl: {
 			new(
-				folders: IWatchRequest[],
 				onChange: (changes: IDiskFileChange[]) => void,
 				onLogMessage: (msg: ILogMessage) => void,
 				verboseLogging: boolean,
@@ -570,7 +569,7 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple
 				if (product.quality === 'stable') {
 					// in stable use legacy for single folder workspaces
 					// TODO@bpasero remove me eventually
-					enableLegacyWatcher = folders.length === 1;
+					enableLegacyWatcher = folders === 1;
 				}
 			}
 
@@ -585,9 +584,7 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple
 			}
 		}
 
-		// Create and start watching
 		return new watcherImpl(
-			folders,
 			changes => onChange(changes),
 			msg => onLogMessage(msg),
 			verboseLogging,

src/vs/platform/files/node/watcher/nsfw/watcherService.ts

@@ -11,15 +11,11 @@ import { IDiskFileChange, ILogMessage, IWatchRequest, WatcherService } from 'vs/
 
 export class FileWatcher extends WatcherService {
 
-	private static readonly MAX_RESTARTS = 5;
-
 	private service: IWatcherService | undefined;
 
 	private isDisposed = false;
-	private restartCounter = 0;
 
 	constructor(
-		private requests: IWatchRequest[],
 		private readonly onDidFilesChange: (changes: IDiskFileChange[]) => void,
 		private readonly onLogMessage: (msg: ILogMessage) => void,
 		private verboseLogging: boolean
@@ -43,20 +39,6 @@ export class FileWatcher extends WatcherService {
 			}
 		));
 
-		this._register(client.onDidProcessExit(() => {
-			// our watcher app should never be completed because it keeps on watching. being in here indicates
-			// that the watcher process died and we want to restart it here. we only do it a max number of times
-			if (!this.isDisposed) {
-				if (this.restartCounter <= FileWatcher.MAX_RESTARTS) {
-					this.error('terminated unexpectedly and is restarted again...');
-					this.restartCounter++;
-					this.startWatching();
-				} else {
-					this.error('failed to start after retrying for some time, giving up. Please report this as a bug report!');
-				}
-			}
-		}));
-
 		// Initialize watcher
 		this.service = ProxyChannel.toService<IWatcherService>(getNextTickChannel(client.getChannel('watcher')));
 		this.service.setVerboseLogging(this.verboseLogging);
@@ -64,9 +46,6 @@ export class FileWatcher extends WatcherService {
 		// Wire in event handlers
 		this._register(this.service.onDidChangeFile(e => !this.isDisposed && this.onDidFilesChange(e)));
 		this._register(this.service.onDidLogMessage(e => this.onLogMessage(e)));
-
-		// Start watching
-		this.watch(this.requests);
 	}
 
 	async setVerboseLogging(verboseLogging: boolean): Promise<void> {
@@ -82,9 +61,9 @@ export class FileWatcher extends WatcherService {
 	}
 
 	async watch(requests: IWatchRequest[]): Promise<void> {
-		this.requests = requests;
-
-		await this.service?.watch(requests);
+		if (!this.isDisposed) {
+			await this.service?.watch(requests);
+		}
 	}
 
 	override dispose(): void {

src/vs/platform/files/node/watcher/parcel/watcherService.ts

@@ -3,6 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { DisposableStore, MutableDisposable } from 'vs/base/common/lifecycle';
 import { FileAccess } from 'vs/base/common/network';
 import { getNextTickChannel, ProxyChannel } from 'vs/base/parts/ipc/common/ipc';
 import { Client } from 'vs/base/parts/ipc/node/ipc.cp';
@@ -13,23 +14,31 @@ export class FileWatcher extends WatcherService {
 	private static readonly MAX_RESTARTS = 5;
 
 	private service: IWatcherService | undefined;
+	private serviceDisposables = this._register(new MutableDisposable());
+
+	private requests: IWatchRequest[] | undefined = undefined;
 
-	private isDisposed = false;
 	private restartCounter = 0;
 
 	constructor(
-		private requests: IWatchRequest[],
 		private readonly onDidFilesChange: (changes: IDiskFileChange[]) => void,
 		private readonly onLogMessage: (msg: ILogMessage) => void,
 		private verboseLogging: boolean
 	) {
 		super();
 
-		this.startWatching();
+		this.init();
 	}
 
-	private startWatching(): void {
-		const client = this._register(new Client(
+	private init(): void {
+
+		// Associate disposables to the service
+		const disposables = new DisposableStore();
+		this.serviceDisposables.value = disposables;
+
+		// Fork the parcel file watcher and build a client around
+		// its server for passing over requests and receiving events.
+		const client = disposables.add(new Client(
 			FileAccess.asFileUri('bootstrap-fork', require).fsPath,
 			{
 				serverName: 'File Watcher (parcel, node.js)',
@@ -42,17 +51,16 @@ export class FileWatcher extends WatcherService {
 			}
 		));
 
-		this._register(client.onDidProcessExit(() => {
-			// our watcher app should never be completed because it keeps on watching. being in here indicates
-			// that the watcher process died and we want to restart it here. we only do it a max number of times
-			if (!this.isDisposed) {
-				if (this.restartCounter <= FileWatcher.MAX_RESTARTS) {
-					this.error('terminated unexpectedly and is restarted again...');
-					this.restartCounter++;
-					this.startWatching();
-				} else {
-					this.error('failed to start after retrying for some time, giving up. Please report this as a bug report!');
-				}
+		disposables.add(client.onDidProcessExit(() => {
+			// Our watcher app should never be completed because it keeps
+			// on watching. being in here indicates that the watcher process
+			// died and we want to restart it here. we only do it a max number
+			// of times
+			if (this.restartCounter <= FileWatcher.MAX_RESTARTS && this.requests) {
+				this.error('terminated unexpectedly and is restarted again...');
+				this.restart(this.requests);
+			} else {
+				this.error('failed to start after retrying for some time, giving up. Please report this as a bug report!');
 			}
 		}));
 
@@ -61,23 +69,16 @@ export class FileWatcher extends WatcherService {
 		this.service.setVerboseLogging(this.verboseLogging);
 
 		// Wire in event handlers
-		this._register(this.service.onDidChangeFile(e => !this.isDisposed && this.onDidFilesChange(e)));
-		this._register(this.service.onDidLogMessage(e => this.onLogMessage(e)));
-
-		// Start watching
-		this.watch(this.requests);
+		disposables.add(this.service.onDidChangeFile(e => this.onDidFilesChange(e)));
+		disposables.add(this.service.onDidLogMessage(e => this.onLogMessage(e)));
 	}
 
-	async setVerboseLogging(verboseLogging: boolean): Promise<void> {
-		this.verboseLogging = verboseLogging;
+	private restart(requests: IWatchRequest[]): void {
+		this.error('terminated unexpectedly and is restarted again...');
+		this.restartCounter++;
 
-		if (!this.isDisposed) {
-			await this.service?.setVerboseLogging(verboseLogging);
-		}
-	}
-
-	error(message: string) {
-		this.onLogMessage({ type: 'error', message: `[File Watcher (parcel)] ${message}` });
+		this.init();
+		this.watch(requests);
 	}
 
 	async watch(requests: IWatchRequest[]): Promise<void> {
@@ -86,9 +87,21 @@ export class FileWatcher extends WatcherService {
 		await this.service?.watch(requests);
 	}
 
-	override dispose(): void {
-		this.isDisposed = true;
+	async setVerboseLogging(verboseLogging: boolean): Promise<void> {
+		this.verboseLogging = verboseLogging;
 
-		super.dispose();
+		await this.service?.setVerboseLogging(verboseLogging);
+	}
+
+	private error(message: string) {
+		this.onLogMessage({ type: 'error', message: `[File Watcher (parcel)] ${message}` });
+	}
+
+	override dispose(): void {
+
+		// Render the serve invalid from here
+		this.service = undefined;
+
+		return super.dispose();
 	}
 }

src/vs/platform/files/node/watcher/unix/watcherService.ts

@@ -21,8 +21,9 @@ export class FileWatcher extends WatcherService {
 	private isDisposed = false;
 	private restartCounter = 0;
 
+	private requests: IWatchRequest[] | undefined = undefined;
+
 	constructor(
-		private requests: IWatchRequest[],
 		private readonly onDidFilesChange: (changes: IDiskFileChange[]) => void,
 		private readonly onLogMessage: (msg: ILogMessage) => void,
 		private verboseLogging: boolean,
@@ -51,10 +52,11 @@ export class FileWatcher extends WatcherService {
 			// our watcher app should never be completed because it keeps on watching. being in here indicates
 			// that the watcher process died and we want to restart it here. we only do it a max number of times
 			if (!this.isDisposed) {
-				if (this.restartCounter <= FileWatcher.MAX_RESTARTS) {
+				if (this.restartCounter <= FileWatcher.MAX_RESTARTS && this.requests) {
 					this.error('terminated unexpectedly and is restarted again...');
 					this.restartCounter++;
 					this.startWatching();
+					this.service?.watch(this.requests);
 				} else {
 					this.error('failed to start after retrying for some time, giving up. Please report this as a bug report!');
 				}
@@ -68,9 +70,6 @@ export class FileWatcher extends WatcherService {
 		// Wire in event handlers
 		this._register(this.service.onDidChangeFile(e => !this.isDisposed && this.onDidFilesChange(e)));
 		this._register(this.service.onDidLogMessage(e => this.onLogMessage(e)));
-
-		// Start watching
-		this.watch(this.requests);
 	}
 
 	async setVerboseLogging(verboseLogging: boolean): Promise<void> {

src/vs/workbench/services/files/electron-sandbox/diskFileSystemProvider.ts

@@ -13,7 +13,7 @@ import { ReadableStreamEvents } from 'vs/base/common/stream';
 import { URI } from 'vs/base/common/uri';
 import { IPCFileSystemProvider } from 'vs/platform/files/common/ipcFileSystemProvider';
 import { IDisposable } from 'vs/base/common/lifecycle';
-import { IDiskFileChange, ILogMessage, IWatchRequest, WatcherService } from 'vs/platform/files/common/watcher';
+import { IDiskFileChange, ILogMessage, WatcherService } from 'vs/platform/files/common/watcher';
 import { ParcelFileWatcher } from 'vs/workbench/services/files/electron-sandbox/parcelWatcherService';
 import { ILogService } from 'vs/platform/log/common/log';
 import { ISharedProcessWorkerWorkbenchService } from 'vs/workbench/services/sharedProcess/electron-sandbox/sharedProcessWorkerWorkbenchService';
@@ -155,13 +155,12 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple
 	}
 
 	protected createRecursiveWatcher(
-		folders: IWatchRequest[],
+		folders: number,
 		onChange: (changes: IDiskFileChange[]) => void,
 		onLogMessage: (msg: ILogMessage) => void,
 		verboseLogging: boolean
 	): WatcherService {
 		return new ParcelFileWatcher(
-			folders,
 			changes => onChange(changes),
 			msg => onLogMessage(msg),
 			verboseLogging,

src/vs/workbench/services/files/electron-sandbox/parcelWatcherService.ts

@@ -12,7 +12,6 @@ export class ParcelFileWatcher extends WatcherService {
 	private readonly service: IWatcherService;
 
 	constructor(
-		requests: IWatchRequest[],
 		private readonly onDidFilesChange: (changes: IDiskFileChange[]) => void,
 		private readonly onLogMessage: (msg: ILogMessage) => void,
 		verboseLogging: boolean,
@@ -20,25 +19,19 @@ export class ParcelFileWatcher extends WatcherService {
 	) {
 		super();
 
-		// Start watching
-		{
-			// Acquire parcel watcher via shared process worker
-			const watcherChannel = this.sharedProcessWorkerWorkbenchService.createWorkerChannel({
-				moduleId: 'vs/platform/files/node/watcher/parcel/watcherApp',
-				type: 'watcherServiceParcelSharedProcess'
-			}, 'watcher').channel;
+		// Acquire parcel watcher via shared process worker
+		const watcherChannel = this.sharedProcessWorkerWorkbenchService.createWorkerChannel({
+			moduleId: 'vs/platform/files/node/watcher/parcel/watcherApp',
+			type: 'watcherServiceParcelSharedProcess'
+		}, 'watcher').channel;
 
-			// Initialize watcher
-			this.service = ProxyChannel.toService<IWatcherService>(watcherChannel);
-			this.service.setVerboseLogging(verboseLogging);
+		// Initialize watcher
+		this.service = ProxyChannel.toService<IWatcherService>(watcherChannel);
+		this.service.setVerboseLogging(verboseLogging);
 
-			// Wire in event handlers
-			this._register(this.service.onDidChangeFile(e => this.onDidFilesChange(e)));
-			this._register(this.service.onDidLogMessage(e => this.onLogMessage(e)));
-
-			// Start watching
-			this.watch(requests);
-		}
+		// Wire in event handlers
+		this._register(this.service.onDidChangeFile(e => this.onDidFilesChange(e)));
+		this._register(this.service.onDidLogMessage(e => this.onLogMessage(e)));
 	}
 
 	setVerboseLogging(verboseLogging: boolean): Promise<void> {