From 81cf536582dcfd31371b48d610613917a875a804 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Fri, 12 Nov 2021 10:52:40 +0100 Subject: [PATCH] watcher - stop sending out delete events on workspace root (#136673) --- src/vs/platform/files/common/watcher.ts | 20 +++++----- .../node/watcher/nodejs/watcherService.ts | 12 +++--- .../node/watcher/nsfw/nsfwWatcherService.ts | 4 +- .../watcher/parcel/parcelWatcherService.ts | 39 ++++++++++++++----- .../node/recursiveWatcher.integrationTest.ts | 33 +++++++++++----- ...lizer.test.ts => watcherCoalescer.test.ts} | 20 +++++----- .../contrib/files/browser/workspaceWatcher.ts | 2 +- 7 files changed, 82 insertions(+), 48 deletions(-) rename src/vs/platform/files/test/node/{watcherNormalizer.test.ts => watcherCoalescer.test.ts} (92%) diff --git a/src/vs/platform/files/common/watcher.ts b/src/vs/platform/files/common/watcher.ts index 4314712e66b..eb1d60df636 100644 --- a/src/vs/platform/files/common/watcher.ts +++ b/src/vs/platform/files/common/watcher.ts @@ -185,20 +185,20 @@ export function toFileChanges(changes: IDiskFileChange[]): IFileChange[] { })); } -export function normalizeFileChanges(changes: IDiskFileChange[]): IDiskFileChange[] { +export function coalesceEvents(changes: IDiskFileChange[]): IDiskFileChange[] { // Build deltas - const normalizer = new EventNormalizer(); + const coalescer = new EventCoalescer(); for (const event of changes) { - normalizer.processEvent(event); + coalescer.processEvent(event); } - return normalizer.normalize(); + return coalescer.coalesce(); } -class EventNormalizer { +class EventCoalescer { - private readonly normalized = new Set(); + private readonly coalesced = new Set(); private readonly mapPathToChange = new Map(); private toKey(event: IDiskFileChange): string { @@ -232,7 +232,7 @@ class EventNormalizer { // Ignore CREATE followed by DELETE in one go else if (currentChangeType === FileChangeType.ADDED && newChangeType === FileChangeType.DELETED) { this.mapPathToChange.delete(this.toKey(event)); - this.normalized.delete(existingEvent); + this.coalesced.delete(existingEvent); } // Flatten DELETE followed by CREATE into CHANGE @@ -255,12 +255,12 @@ class EventNormalizer { } if (keepEvent) { - this.normalized.add(event); + this.coalesced.add(event); this.mapPathToChange.set(this.toKey(event), event); } } - normalize(): IDiskFileChange[] { + coalesce(): IDiskFileChange[] { const addOrChangeEvents: IDiskFileChange[] = []; const deletedPaths: string[] = []; @@ -271,7 +271,7 @@ class EventNormalizer { // 1.) split ADD/CHANGE and DELETED events // 2.) sort short deleted paths to the top // 3.) for each DELETE, check if there is a deleted parent and ignore the event in that case - return Array.from(this.normalized).filter(e => { + return Array.from(this.coalesced).filter(e => { if (e.type !== FileChangeType.DELETED) { addOrChangeEvents.push(e); diff --git a/src/vs/platform/files/node/watcher/nodejs/watcherService.ts b/src/vs/platform/files/node/watcher/nodejs/watcherService.ts index 346cdb0b656..983b291e229 100644 --- a/src/vs/platform/files/node/watcher/nodejs/watcherService.ts +++ b/src/vs/platform/files/node/watcher/nodejs/watcherService.ts @@ -10,7 +10,7 @@ import { realpath } from 'vs/base/node/extpath'; import { SymlinkSupport } from 'vs/base/node/pfs'; import { CHANGE_BUFFER_DELAY, watchFile, watchFolder } from 'vs/base/node/watcher'; import { FileChangeType } from 'vs/platform/files/common/files'; -import { IDiskFileChange, ILogMessage, normalizeFileChanges } from 'vs/platform/files/common/watcher'; +import { IDiskFileChange, ILogMessage, coalesceEvents } from 'vs/platform/files/common/watcher'; export class FileWatcher extends Disposable { private isDisposed: boolean | undefined; @@ -95,19 +95,19 @@ export class FileWatcher extends Disposable { const fileChanges = this.fileChangesBuffer; this.fileChangesBuffer = []; - // Event normalization - const normalizedFileChanges = normalizeFileChanges(fileChanges); + // Event coalsecer + const coalescedFileChanges = coalesceEvents(fileChanges); // Logging if (this.verboseLogging) { - for (const event of normalizedFileChanges) { + for (const event of coalescedFileChanges) { this.onVerbose(`>> normalized ${event.type === FileChangeType.ADDED ? '[ADDED]' : event.type === FileChangeType.DELETED ? '[DELETED]' : '[CHANGED]'} ${event.path}`); } } // Fire - if (normalizedFileChanges.length > 0) { - this.onDidFilesChange(normalizedFileChanges); + if (coalescedFileChanges.length > 0) { + this.onDidFilesChange(coalescedFileChanges); } }); } diff --git a/src/vs/platform/files/node/watcher/nsfw/nsfwWatcherService.ts b/src/vs/platform/files/node/watcher/nsfw/nsfwWatcherService.ts index 0b7bd2a804a..7b415d8b902 100644 --- a/src/vs/platform/files/node/watcher/nsfw/nsfwWatcherService.ts +++ b/src/vs/platform/files/node/watcher/nsfw/nsfwWatcherService.ts @@ -18,7 +18,7 @@ import { isMacintosh } from 'vs/base/common/platform'; import { realcaseSync, realpathSync } from 'vs/base/node/extpath'; import { FileChangeType } from 'vs/platform/files/common/files'; import { IWatcherService } from 'vs/platform/files/node/watcher/nsfw/watcher'; -import { IDiskFileChange, ILogMessage, normalizeFileChanges, IWatchRequest } from 'vs/platform/files/common/watcher'; +import { IDiskFileChange, ILogMessage, coalesceEvents, IWatchRequest } from 'vs/platform/files/common/watcher'; import { watchFolder } from 'vs/base/node/watcher'; interface IWatcher extends IDisposable { @@ -192,7 +192,7 @@ export class NsfwWatcherService extends Disposable implements IWatcherService { undeliveredFileEvents = []; // Broadcast to clients normalized - const normalizedEvents = normalizeFileChanges(this.normalizeEvents(undeliveredFileEventsToEmit, request, realBasePathDiffers, realBasePathLength)); + const normalizedEvents = coalesceEvents(this.normalizeEvents(undeliveredFileEventsToEmit, request, realBasePathDiffers, realBasePathLength)); this.emitEvents(normalizedEvents); }, this.getOptions(watcher)).then(async nsfwWatcher => { diff --git a/src/vs/platform/files/node/watcher/parcel/parcelWatcherService.ts b/src/vs/platform/files/node/watcher/parcel/parcelWatcherService.ts index a731395f51b..63a859ec83d 100644 --- a/src/vs/platform/files/node/watcher/parcel/parcelWatcherService.ts +++ b/src/vs/platform/files/node/watcher/parcel/parcelWatcherService.ts @@ -22,7 +22,7 @@ import { generateUuid } from 'vs/base/common/uuid'; import { realcaseSync, realpathSync } from 'vs/base/node/extpath'; import { watchFolder } from 'vs/base/node/watcher'; import { FileChangeType } from 'vs/platform/files/common/files'; -import { IDiskFileChange, ILogMessage, normalizeFileChanges, IWatchRequest, IWatcherService } from 'vs/platform/files/common/watcher'; +import { IDiskFileChange, ILogMessage, coalesceEvents, IWatchRequest, IWatcherService } from 'vs/platform/files/common/watcher'; export interface IWatcher { @@ -377,14 +377,19 @@ export class ParcelWatcherService extends Disposable implements IWatcherService // Check for excludes const rawEvents = this.handleExcludes(parcelEvents, excludes); - // Normalize and detect root path deletes + // Normalize events: handle NFC normalization and symlinks const { events: normalizedEvents, rootDeleted } = this.normalizeEvents(rawEvents, watcher.request, realPathDiffers, realPathLength); - // Broadcast to clients coalesced - const coalescedEvents = normalizeFileChanges(normalizedEvents); - this.emitEvents(coalescedEvents); + // Coalesce events: merge events of same kind + const coalescedEvents = coalesceEvents(normalizedEvents); - // Handle root path delete if confirmed from coalseced events + // Filter events: check for specific events we want to exclude + const filteredEvents = this.filterEvents(coalescedEvents, watcher.request, rootDeleted); + + // Broadcast to clients + this.emitEvents(filteredEvents); + + // Handle root path delete if confirmed from coalesced events if (rootDeleted && coalescedEvents.some(event => event.path === watcher.request.path && event.type === FileChangeType.DELETED)) { this.onWatchedPathDeleted(watcher); } @@ -485,6 +490,25 @@ export class ParcelWatcherService extends Disposable implements IWatcherService return { events, rootDeleted }; } + private filterEvents(events: IDiskFileChange[], request: IWatchRequest, rootDeleted: boolean): IDiskFileChange[] { + if (!rootDeleted) { + return events; + } + + return events.filter(event => { + if (event.path === request.path && event.type === FileChangeType.DELETED) { + // Explicitly exclude changes to root if we have any + // to avoid VS Code closing all opened editors which + // can happen e.g. in case of network connectivity + // issues + // (https://github.com/microsoft/vscode/issues/136673) + return false; + } + + return true; + }); + } + private onWatchedPathDeleted(watcher: IWatcher): void { this.warn('Watcher shutdown because watched path got deleted', watcher); @@ -502,9 +526,6 @@ export class ParcelWatcherService extends Disposable implements IWatcherService // Stop watching that parent folder disposable.dispose(); - // Send a manual event given we know the root got added again - this.emitEvents([{ path: watcher.request.path, type: FileChangeType.ADDED }]); - // Restart the file watching this.restartWatching(watcher); } diff --git a/src/vs/platform/files/test/node/recursiveWatcher.integrationTest.ts b/src/vs/platform/files/test/node/recursiveWatcher.integrationTest.ts index ee26d0ec595..9237d7b6351 100644 --- a/src/vs/platform/files/test/node/recursiveWatcher.integrationTest.ts +++ b/src/vs/platform/files/test/node/recursiveWatcher.integrationTest.ts @@ -101,13 +101,13 @@ flakySuite('Recursive Watcher (parcel)', () => { } } - async function awaitEvent(service: TestParcelWatcherService, path: string, type: FileChangeType, failOnEventReason?: string): Promise { + function awaitEvent(service: TestParcelWatcherService, path: string, type: FileChangeType, failOnEventReason?: string): Promise { if (loggingEnabled) { console.log(`Awaiting change type '${toMsg(type)}' on file '${path}'`); } // Await the event - await new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { const disposable = service.onDidChangeFile(events => { for (const event of events) { if (event.path === path && event.type === type) { @@ -124,6 +124,22 @@ flakySuite('Recursive Watcher (parcel)', () => { }); } + function awaitMessage(service: TestParcelWatcherService, type: 'trace' | 'warn' | 'error' | 'info' | 'debug'): Promise { + if (loggingEnabled) { + console.log(`Awaiting message of type ${type}`); + } + + // Await the message + return new Promise(resolve => { + const disposable = service.onDidLogMessage(msg => { + if (msg.type === type) { + disposable.dispose(); + resolve(); + } + }); + }); + } + test('basics', async function () { await service.watch([{ path: testDir, excludes: [] }]); @@ -447,22 +463,19 @@ flakySuite('Recursive Watcher (parcel)', () => { await service.watch([{ path: watchedPath, excludes: [] }]); - // Delete watched path - let changeFuture: Promise = awaitEvent(service, watchedPath, FileChangeType.DELETED); + // Delete watched path and await + const warnFuture = awaitMessage(service, 'warn'); await Promises.rm(watchedPath, RimRafMode.UNLINK); - await changeFuture; + await warnFuture; // Restore watched path - changeFuture = awaitEvent(service, watchedPath, FileChangeType.ADDED); await Promises.mkdir(watchedPath); - await changeFuture; - - await timeout(20); // restart is delayed + await timeout(200); // restart is delayed await service.whenReady(); // Verify events come in again const newFilePath = join(watchedPath, 'newFile.txt'); - changeFuture = awaitEvent(service, newFilePath, FileChangeType.ADDED); + const changeFuture = awaitEvent(service, newFilePath, FileChangeType.ADDED); await Promises.writeFile(newFilePath, 'Hello World'); await changeFuture; }); diff --git a/src/vs/platform/files/test/node/watcherNormalizer.test.ts b/src/vs/platform/files/test/node/watcherCoalescer.test.ts similarity index 92% rename from src/vs/platform/files/test/node/watcherNormalizer.test.ts rename to src/vs/platform/files/test/node/watcherCoalescer.test.ts index 988be68c599..7853f6ce565 100644 --- a/src/vs/platform/files/test/node/watcherNormalizer.test.ts +++ b/src/vs/platform/files/test/node/watcherCoalescer.test.ts @@ -9,7 +9,7 @@ import { isLinux, isWindows } from 'vs/base/common/platform'; import { isEqual } from 'vs/base/common/resources'; import { URI as uri } from 'vs/base/common/uri'; import { FileChangesEvent, FileChangeType, IFileChange } from 'vs/platform/files/common/files'; -import { IDiskFileChange, normalizeFileChanges, toFileChanges } from 'vs/platform/files/common/watcher'; +import { IDiskFileChange, coalesceEvents, toFileChanges } from 'vs/platform/files/common/watcher'; class TestFileWatcher { private readonly _onDidFilesChange: Emitter<{ raw: IFileChange[], event: FileChangesEvent }>; @@ -28,12 +28,12 @@ class TestFileWatcher { private onRawFileEvents(events: IDiskFileChange[]): void { - // Normalize - let normalizedEvents = normalizeFileChanges(events); + // Coalesce + let coalescedEvents = coalesceEvents(events); // Emit through event emitter - if (normalizedEvents.length > 0) { - this._onDidFilesChange.fire({ raw: toFileChanges(normalizedEvents), event: this.toFileChangesEvent(normalizedEvents) }); + if (coalescedEvents.length > 0) { + this._onDidFilesChange.fire({ raw: toFileChanges(coalescedEvents), event: this.toFileChangesEvent(coalescedEvents) }); } } @@ -118,7 +118,7 @@ suite('Watcher Events Normalizer', () => { }); }); - test('event normalization: ignore CREATE followed by DELETE', done => { + test('event coalescer: ignore CREATE followed by DELETE', done => { const watch = new TestFileWatcher(); const created = uri.file('/users/data/src/related'); @@ -143,7 +143,7 @@ suite('Watcher Events Normalizer', () => { watch.report(raw); }); - test('event normalization: flatten DELETE followed by CREATE into CHANGE', done => { + test('event coalescer: flatten DELETE followed by CREATE into CHANGE', done => { const watch = new TestFileWatcher(); const deleted = uri.file('/users/data/src/related'); @@ -169,7 +169,7 @@ suite('Watcher Events Normalizer', () => { watch.report(raw); }); - test('event normalization: ignore UPDATE when CREATE received', done => { + test('event coalescer: ignore UPDATE when CREATE received', done => { const watch = new TestFileWatcher(); const created = uri.file('/users/data/src/related'); @@ -196,7 +196,7 @@ suite('Watcher Events Normalizer', () => { watch.report(raw); }); - test('event normalization: apply DELETE', done => { + test('event coalescer: apply DELETE', done => { const watch = new TestFileWatcher(); const updated = uri.file('/users/data/src/related'); @@ -225,7 +225,7 @@ suite('Watcher Events Normalizer', () => { watch.report(raw); }); - test('event normalization: track case renames', done => { + test('event coalescer: track case renames', done => { const watch = new TestFileWatcher(); const oldPath = uri.file('/users/data/src/added'); diff --git a/src/vs/workbench/contrib/files/browser/workspaceWatcher.ts b/src/vs/workbench/contrib/files/browser/workspaceWatcher.ts index 90f7bba43b4..68fc90c3a16 100644 --- a/src/vs/workbench/contrib/files/browser/workspaceWatcher.ts +++ b/src/vs/workbench/contrib/files/browser/workspaceWatcher.ts @@ -90,7 +90,7 @@ export class WorkspaceWatcher extends Disposable { else if (msg.indexOf('EUNKNOWN') >= 0) { this.notificationService.prompt( Severity.Warning, - localize('eshutdownError', "File changes watcher stopped unexpectedly. Please reload the window to enable the watcher again."), + localize('eshutdownError', "File changes watcher stopped unexpectedly. A reload of the window may enable the watcher again unless the workspace cannot be watched for file changes."), [{ label: localize('reload', "Reload"), run: () => this.hostService.reload()