From 0975de98de1b9285548bee7b14797cb3e803414b Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 14 Nov 2016 08:38:05 +0100 Subject: [PATCH] External file watcher fails for editors that do atomic saves (fixes #15111) --- .../services/files/node/fileService.ts | 47 ++++++++++++++----- .../files/test/node/fileService.test.ts | 24 +++++++++- 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/vs/workbench/services/files/node/fileService.ts b/src/vs/workbench/services/files/node/fileService.ts index 53c3445a6ee..735612e9698 100644 --- a/src/vs/workbench/services/files/node/fileService.ts +++ b/src/vs/workbench/services/files/node/fileService.ts @@ -23,6 +23,7 @@ import extfs = require('vs/base/node/extfs'); import { nfcall, Limiter, ThrottledDelayer } from 'vs/base/common/async'; import uri from 'vs/base/common/uri'; import nls = require('vs/nls'); +import { isWindows } from 'vs/base/common/platform'; import pfs = require('vs/base/node/pfs'); import encoding = require('vs/base/node/encoding'); @@ -70,6 +71,7 @@ export class FileService implements IFileService { public _serviceBrand: any; private static FS_EVENT_DELAY = 50; // aggregate and only emit events when changes have stopped for this duration (in ms) + private static FS_REWATCH_DELAY = 300; // delay to rewatch a file that was renamed or deleted (in ms) private static MAX_DEGREE_OF_PARALLEL_FS_OPS = 10; // degree of parallel fs calls that we accept at the same time private basePath: string; @@ -100,12 +102,12 @@ export class FileService implements IFileService { this.options = options || Object.create(null); this.tmpPath = this.options.tmpDir || os.tmpdir(); - if (this.options && !this.options.errorLogger) { + if (!this.options.errorLogger) { this.options.errorLogger = console.error; } if (this.basePath && !this.options.disableWatcher) { - if (process.platform === 'win32') { + if (isWindows) { this.setupWin32WorkspaceWatching(); } else { this.setupUnixWorkspaceWatching(); @@ -596,26 +598,44 @@ export class FileService implements IFileService { } public watchFileChanges(resource: uri): void { - assert.ok(resource && resource.scheme === 'file', 'Invalid resource for watching: ' + resource); - - const fsPath = resource.fsPath; + assert.ok(resource && resource.scheme === 'file', `Invalid resource for watching: ${resource}`); // Create or get watcher for provided path let watcher = this.activeFileChangesWatchers[resource.toString()]; if (!watcher) { + const fsPath = resource.fsPath; + try { watcher = fs.watch(fsPath); // will be persistent but not recursive } catch (error) { - // the path might not exist anymore, ignore this error and return - return; + return; // the path might not exist anymore, ignore this error and return } this.activeFileChangesWatchers[resource.toString()] = watcher; // eventType is either 'rename' or 'change' - watcher.on('change', (eventType: string) => { - if (eventType !== 'change') { - return; // only care about changes for now ('rename' is not reliable and can be send even if the file is still there with some tools) + const fsName = paths.basename(resource.fsPath); + watcher.on('change', (eventType: string, filename: string) => { + const renamedOrDeleted = ((filename && filename !== fsName) || eventType === 'rename'); + + // The file was either deleted or renamed. Many tools apply changes to files in an + // atomic way ("Atomic Save") by first renaming the file to a temporary name and then + // renaming it back to the original name. Our watcher will detect this as a rename + // and then stops to work on Mac and Linux because the watcher is applied to the + // inode and not the name. The fix is to detect this case and trying to watch the file + // again after a certain delay. + if (renamedOrDeleted) { + if (isWindows) { + return; // on Windows the watcher continues to work even if the file was saved atomically with a replace operation + } + + // Very important to dispose the watcher which now points to a stale inode + this.unwatchFileChanges(resource); + + // Wait a bit and try to install watcher again, assuming that the file was renamed quickly ("Atomic Save") + setTimeout(() => { + this.watchFileChanges(resource); + }, FileService.FS_REWATCH_DELAY); } // add to bucket of undelivered events @@ -638,6 +658,11 @@ export class FileService implements IFileService { return TPromise.as(null); }); }); + + // Errors + watcher.on('error', (error) => { + this.options.errorLogger(error); + }); } } @@ -819,4 +844,4 @@ export class StatResolver { }); }); } -} \ No newline at end of file +} diff --git a/src/vs/workbench/services/files/test/node/fileService.test.ts b/src/vs/workbench/services/files/test/node/fileService.test.ts index 9010079c455..f599f874ba4 100644 --- a/src/vs/workbench/services/files/test/node/fileService.test.ts +++ b/src/vs/workbench/services/files/test/node/fileService.test.ts @@ -481,6 +481,28 @@ suite('FileService', () => { }, 100); }); + test('watchFileChanges - support atomic save', function (done: () => void) { + let toWatch = uri.file(path.join(testDir, 'index.html')); + + service.watchFileChanges(toWatch); + + events.addListener2(EventType.FILE_CHANGES, (e: FileChangesEvent) => { + assert.ok(e); + + service.unwatchFileChanges(toWatch); + done(); + }); + + setTimeout(() => { + // Simulate atomic save by deleting the file, creating it under different name + // and then replacing the previously deleted file with those contents + const renamed = `${toWatch.fsPath}.bak`; + fs.unlinkSync(toWatch.fsPath); + fs.writeFileSync(renamed, 'Changes'); + fs.renameSync(renamed, toWatch.fsPath); + }, 100); + }); + test('options - encoding', function (done: () => void) { // setup @@ -564,4 +586,4 @@ suite('FileService', () => { }); }); }); -}); \ No newline at end of file +}); -- GitLab