1.7: TS Server opens a file handle for watching for each file in the project results in EMFILE error

[bpasero]

Since we updated from TS server 1.6 to 1.7 we notice sporadic crashes in the TS server for our VS Code code base that has around 1100 typescript files in one project.

When I compare 1.6 with 1.7 I notice that you changed the strategy for watching for file changes from fs.watchFile to fs.watch. The latter is recommended because it leverages native file watching while the former is polling for changes (see https://github.com/Microsoft/TypeScript/blob/master/src/compiler/sys.ts#L409 in 1.7 and https://github.com/Microsoft/TypeScript/blob/v1.6.2/src/compiler/sys.ts#L286 in 1.6).

Now, since this method is being used for every file of the project, on Unix/Mac you end up with as much open file handles as you have TS files in the project. For VS Code this means > 1000 file handles and eventually I see this in the logs:

EMFILE: too many open files, open '/Users/bpasero/Development/monaco/src/vs/workbench/services/files/node/watcher/unix/chokidarWatcherService.ts' Error: EMFILE: too many open files, open '/Users/bpasero/Development/monaco/src/vs/workbench/services/files/node/watcher/unix/chokidarWatcherService.ts' at Error (native) at Object.fs.openSync (fs.js:549:18) at Object.module.(anonymous function) [as openSync] (ATOM_SHELL_ASAR.js:137:20) at Object.fs.readFileSync (fs.js:397:15) at Object.fs.readFileSync (ATOM_SHELL_ASAR.js:385:29) at Object.readFile (/Users/bpasero/Development/tssdk/built/local/tsserver.js:1904:34) at ProjectService.openFile (/Users/bpasero/Development/tssdk/built/local/tsserver.js:53513:60) at ProjectService.openConfigFile (/Users/bpasero/Development/tssdk/built/local/tsserver.js:53719:45) at ProjectService.openOrUpdateConfiguredProjectForFile (/Users/bpasero/Development/tssdk/built/local/tsserver.js:53583:49) at ProjectService.openClientFile (/Users/bpasero/Development/tssdk/built/local/tsserver.js:53564:22)

I don't see a good work around on my end and will have to go back to 1.6. I think a better solution would be to install a watcher per root directory and then do the filtering on the event instead of installing a watcher for each file upfront.

[mhegazy]

looks like we need to revert to the polling based watch mechanism.

[zhengbli]

@bpasero there is a fix in #6026, can you give it a try? It uses directory watchers instead of single file watchers. Thanks.

[bpasero]

@zhengbli I am using it for this morning and the issue seems fixed! If this change is too risky for 1.7.5, then at least please restore the polling mechanism.

[zhengbli]

@bpasero It might be safer to use polling for all cases for now. I reverted the mechanism in #6026, if you could give it a final try and I'll check the fix in. I'll send out a separate PR after the release.

[zhengbli]

Should be fixed now by #6066.
A non-polling fix will be updated later at #6026.

[zhengbli]

@bpasero can you confirm? Thanks!

[bpasero]

@zhengbli I am a bit confused, this is not the solution used in 1.6 right? Why the hassle of making a change like this if you end up with the same polling?

[bpasero]

Ah sorry, I did not realize that the watched file set was introduced during 1.7 already. I will give it a try today, thanks!