Skip to content

Commit

Permalink
watcher - stop sending out delete events on workspace root (#136673)
Browse files Browse the repository at this point in the history
  • Loading branch information
bpasero committed Nov 12, 2021
1 parent 8d3536c commit 81cf536
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 48 deletions.
20 changes: 10 additions & 10 deletions src/vs/platform/files/common/watcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<IDiskFileChange>();
private readonly coalesced = new Set<IDiskFileChange>();
private readonly mapPathToChange = new Map<string, IDiskFileChange>();

private toKey(event: IDiskFileChange): string {
Expand Down Expand Up @@ -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
Expand All @@ -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[] = [];

Expand All @@ -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);

Expand Down
12 changes: 6 additions & 6 deletions src/vs/platform/files/node/watcher/nodejs/watcherService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
});
}
Expand Down
4 changes: 2 additions & 2 deletions src/vs/platform/files/node/watcher/nsfw/nsfwWatcherService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 => {

Expand Down
39 changes: 30 additions & 9 deletions src/vs/platform/files/node/watcher/parcel/parcelWatcherService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);

Expand All @@ -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);
}
Expand Down
33 changes: 23 additions & 10 deletions src/vs/platform/files/test/node/recursiveWatcher.integrationTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,13 @@ flakySuite('Recursive Watcher (parcel)', () => {
}
}

async function awaitEvent(service: TestParcelWatcherService, path: string, type: FileChangeType, failOnEventReason?: string): Promise<void> {
function awaitEvent(service: TestParcelWatcherService, path: string, type: FileChangeType, failOnEventReason?: string): Promise<void> {
if (loggingEnabled) {
console.log(`Awaiting change type '${toMsg(type)}' on file '${path}'`);
}

// Await the event
await new Promise<void>((resolve, reject) => {
return new Promise<void>((resolve, reject) => {
const disposable = service.onDidChangeFile(events => {
for (const event of events) {
if (event.path === path && event.type === type) {
Expand All @@ -124,6 +124,22 @@ flakySuite('Recursive Watcher (parcel)', () => {
});
}

function awaitMessage(service: TestParcelWatcherService, type: 'trace' | 'warn' | 'error' | 'info' | 'debug'): Promise<void> {
if (loggingEnabled) {
console.log(`Awaiting message of type ${type}`);
}

// Await the message
return new Promise<void>(resolve => {
const disposable = service.onDidLogMessage(msg => {
if (msg.type === type) {
disposable.dispose();
resolve();
}
});
});
}

test('basics', async function () {
await service.watch([{ path: testDir, excludes: [] }]);

Expand Down Expand Up @@ -447,22 +463,19 @@ flakySuite('Recursive Watcher (parcel)', () => {

await service.watch([{ path: watchedPath, excludes: [] }]);

// Delete watched path
let changeFuture: Promise<unknown> = 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;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }>;
Expand All @@ -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) });
}
}

Expand Down Expand Up @@ -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');
Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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');
Expand Down Expand Up @@ -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');
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/files/browser/workspaceWatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 81cf536

Please sign in to comment.