fix: use self instead of this inside FSWatcher onchange callback

[shiftkey]

To reproduce this behaviour on Electron 3.1.x, I have a test project setup (on Windows, but I don't think that matters here):

$ git clone https://github.com/shiftkey/electron-quick-start -b repro-fswatcher-this-issue
$ npm install
$ npm start

Open the dev tools, click the Click me! button, see the app break into your code:

Browse to the source of fs.js and put a break point inside FSWatcher's this._handle.onchange` callback (line 1382):

Resume the app and it should hit the fs.js breakpoint. Check the local scope:

Note that this inside the callback is the FSEvent associated with the onchange event, not the current FSWatcher. Further down the code uses self.* to emit events, and I think this was just a code path that rarely gets exercised.

I haven't triggered the specific code path because it requires the underlying filesystem to return a negative status value, but I know of these two issues in the wild that can be traced to this:

• TypeError: Cannot read property 'close' of undefined at FSEvent.FSWatcher._handle.onchange desktop/desktop#7127
• High CPU Usage on Windows with Mapped Drive when leaving VS Code Open for a while microsoft/vscode#70566

Let me know if this isn't the right branch to target for the PR, and if there's anything else I can do to expedite this fix into the main Electron repository.

[shiftkey]

A quick check of later versions of Electron and this code has been moved (with the self removed):

node/lib/internal/fs/watchers.js

Lines 111 to 132 in 8bc5d17

	this._handle.onchange = (status, eventType, filename) => {
	// TODO(joyeecheung): we may check self._handle.initialized here
	// and return if that is false. This allows us to avoid firing the event
	// after the handle is closed, and to fire both UV_RENAME and UV_CHANGE
	// if they are set by libuv at the same time.
	if (status < 0) {
	if (this._handle !== null) {
	// We don't use this.close() here to avoid firing the close event.
	this._handle.close();
	this._handle = null; // make the handle garbage collectable
	}
	const error = errors.uvException({
	errno: status,
	syscall: 'watch',
	path: filename
	});
	error.filename = filename;
	this.emit('error', error);
	} else {
	this.emit('change', eventType, filename);
	}
	};

I tested out the same repro on 4.1.1 (Node 10.11.0) and it looks like this is correctly set to the FSWatcher, so no need for more forward-porting of this fix:

[MarshallOfSound]

😢 Ah self, how I don't miss you now we have arrow functions 😀

Fix looks good to me 👍

[codebytere]

thanks for catching this @shiftkey!

[bpasero]

@shiftkey awesome description and PR, looking forward to this landing in Electron 3.x 👍

[ckerr]

Nice detective work @shiftkey! Thanks for the patch!