| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent b4b7d36 commit c09bfd8
4 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -325,7 +325,8 @@ fs.watch('./tmp', { encoding: 'buffer' }, (eventType, filename) => { | |||
| 325 | 325 | added: v10.0.0 | |
| 326 | 326 | --> | |
| 327 | 327 | ||
| 328 | - Emitted when the watcher stops watching for changes. | ||
| 328 | + Emitted when the watcher stops watching for changes. The closed | ||
| 329 | + `fs.FSWatcher` object is no longer usable in the event handler. | ||
| 329 | 330 | ||
| 330 | 331 | ### Event: 'error' | |
| 331 | 332 | <!-- YAML | |
@@ -334,7 +335,8 @@ added: v0.5.8 | |||
| 334 | 335 | ||
| 335 | 336 | * `error` {Error} | |
| 336 | 337 | ||
| 337 | - Emitted when an error occurs while watching the file. | ||
| 338 | + Emitted when an error occurs while watching the file. The errored | ||
| 339 | + `fs.FSWatcher` object is no longer usable in the event handler. | ||
| 338 | 340 | ||
| 339 | 341 | ### watcher.close() | |
| 340 | 342 | <!-- YAML | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -100,7 +100,11 @@ function FSWatcher() { | |||
| 100 | 100 | // after the handle is closed, and to fire both UV_RENAME and UV_CHANGE | |
| 101 | 101 | // if they are set by libuv at the same time. | |
| 102 | 102 | if (status < 0) { | |
| 103 | - this._handle.close(); | ||
| 103 | + if (this._handle !== null) { | ||
| 104 | + // We don't use this.close() here to avoid firing the close event. | ||
| 105 | + this._handle.close(); | ||
| 106 | + this._handle = null; // make the handle garbage collectable | ||
| 107 | + } | ||
| 104 | 108 | const error = errors.uvException({ | |
| 105 | 109 | errno: status, | |
| 106 | 110 | syscall: 'watch', | |
@@ -120,13 +124,17 @@ util.inherits(FSWatcher, EventEmitter); | |||
| 120 | 124 | // 1. Throw an Error if it's the first time .start() is called | |
| 121 | 125 | // 2. Return silently if .start() has already been called | |
| 122 | 126 | // on a valid filename and the wrap has been initialized | |
| 127 | + // 3. Return silently if the watcher has already been closed | ||
| 123 | 128 | // This method is a noop if the watcher has already been started. | |
| 124 | 129 | FSWatcher.prototype.start = function(filename, | |
| 125 | 130 | persistent, | |
| 126 | 131 | recursive, | |
| 127 | 132 | encoding) { | |
| 133 | + if (this._handle === null) { // closed | ||
| 134 | + return; | ||
| 135 | + } | ||
| 128 | 136 | assert(this._handle instanceof FSEvent, 'handle must be a FSEvent'); | |
| 129 | - if (this._handle.initialized) { | ||
| 137 | + if (this._handle.initialized) { // already started | ||
| 130 | 138 | return; | |
| 131 | 139 | } | |
| 132 | 140 | ||
@@ -148,13 +156,18 @@ FSWatcher.prototype.start = function(filename, | |||
| 148 | 156 | } | |
| 149 | 157 | }; | |
| 150 | 158 | ||
| 151 | - // This method is a noop if the watcher has not been started. | ||
| 159 | + // This method is a noop if the watcher has not been started or | ||
| 160 | + // has already been closed. | ||
| 152 | 161 | FSWatcher.prototype.close = function() { | |
| 162 | + if (this._handle === null) { // closed | ||
| 163 | + return; | ||
| 164 | + } | ||
| 153 | 165 | assert(this._handle instanceof FSEvent, 'handle must be a FSEvent'); | |
| 154 | - if (!this._handle.initialized) { | ||
| 166 | + if (!this._handle.initialized) { // not started | ||
| 155 | 167 | return; | |
| 156 | 168 | } | |
| 157 | 169 | this._handle.close(); | |
| 170 | + this._handle = null; // make the handle garbage collectable | ||
| 158 | 171 | process.nextTick(emitCloseNT, this); | |
| 159 | 172 | }; | |
| 160 | 173 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,38 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + | ||
| 3 | + // This tests that closing a watcher when the underlying handle is | ||
| 4 | + // already destroyed will result in a noop instead of a crash. | ||
| 5 | + | ||
| 6 | + const common = require('../common'); | ||
| 7 | + const tmpdir = require('../common/tmpdir'); | ||
| 8 | + const fs = require('fs'); | ||
| 9 | + const path = require('path'); | ||
| 10 | + | ||
| 11 | + tmpdir.refresh(); | ||
| 12 | + const root = path.join(tmpdir.path, 'watched-directory'); | ||
| 13 | + fs.mkdirSync(root); | ||
| 14 | + | ||
| 15 | + const watcher = fs.watch(root, { persistent: false, recursive: false }); | ||
| 16 | + | ||
| 17 | + // The following listeners may or may not be invoked. | ||
| 18 | + | ||
| 19 | + watcher.addListener('error', () => { | ||
| 20 | + setTimeout( | ||
| 21 | + () => { watcher.close(); }, // Should not crash if it's invoked | ||
| 22 | + common.platformTimeout(10) | ||
| 23 | + ); | ||
| 24 | + }); | ||
| 25 | + | ||
| 26 | + watcher.addListener('change', () => { | ||
| 27 | + setTimeout( | ||
| 28 | + () => { watcher.close(); }, | ||
| 29 | + common.platformTimeout(10) | ||
| 30 | + ); | ||
| 31 | + }); | ||
| 32 | + | ||
| 33 | + fs.rmdirSync(root); | ||
| 34 | + // Wait for the listener to hit | ||
| 35 | + setTimeout( | ||
| 36 | + common.mustCall(() => {}), | ||
| 37 | + common.platformTimeout(100) | ||
| 38 | + ); | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -46,15 +46,20 @@ for (const testCase of cases) { | |||
| 46 | 46 | fs.writeFileSync(testCase.filePath, content1); | |
| 47 | 47 | ||
| 48 | 48 | let interval; | |
| 49 | - const watcher = fs.watch(testCase[testCase.field]); | ||
| 49 | + const pathToWatch = testCase[testCase.field]; | ||
| 50 | + const watcher = fs.watch(pathToWatch); | ||
| 50 | 51 | watcher.on('error', (err) => { | |
| 51 | 52 | if (interval) { | |
| 52 | 53 | clearInterval(interval); | |
| 53 | 54 | interval = null; | |
| 54 | 55 | } | |
| 55 | 56 | assert.fail(err); | |
| 56 | 57 | }); | |
| 57 | - watcher.on('close', common.mustCall()); | ||
| 58 | + watcher.on('close', common.mustCall(() => { | ||
| 59 | + watcher.close(); // Closing a closed watcher should be a noop | ||
| 60 | + // Starting a closed watcher should be a noop | ||
| 61 | + watcher.start(); | ||
| 62 | + })); | ||
| 58 | 63 | watcher.on('change', common.mustCall(function(eventType, argFilename) { | |
| 59 | 64 | if (interval) { | |
| 60 | 65 | clearInterval(interval); | |
@@ -66,10 +71,16 @@ for (const testCase of cases) { | |||
| 66 | 71 | assert.strictEqual(eventType, 'change'); | |
| 67 | 72 | assert.strictEqual(argFilename, testCase.fileName); | |
| 68 | 73 | ||
| 69 | - watcher.start(); // Starting a started watcher should be a noop | ||
| 70 | - // End of test case | ||
| 74 | + // Starting a started watcher should be a noop | ||
| 75 | + watcher.start(); | ||
| 76 | + watcher.start(pathToWatch); | ||
| 77 | + | ||
| 71 | 78 | watcher.close(); | |
| 79 | + | ||
| 80 | + // We document that watchers cannot be used anymore when it's closed, | ||
| 81 | + // here we turn the methods into noops instead of throwing | ||
| 72 | 82 | watcher.close(); // Closing a closed watcher should be a noop | |
| 83 | + watcher.start(); // Starting a closed watcher should be a noop | ||
| 73 | 84 | })); | |
| 74 | 85 | ||
| 75 | 86 | // Long content so it's actually flushed. toUpperCase so there's real change. | |
| Back | FazBrowse Home | New Git URL |
0 commit comments