Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
Description
As part of supporting IDE runs we use TS "watch programs" to allow efficient recomputation of the TS program as the IDE sends us updated code for files.
However, as we also need to simultaneously support CLI runs we cannot allow TS to actually attach file watchers to the filesystem. File watchers count as open pipes - which will cause NodeJS to prevent ESLint from ever exiting (making it seem like ESLint has "hung").
So in order for us to "get the best of both worlds", we take the callback functions that TS uses to be alerted of filesystem changes and we store them in a map of sets (keyed by the file path).
typescript-eslint/packages/typescript-estree/src/create-program/createWatchProgram.ts
Lines 24 to 35 inb3ac5f6
/** | |
* Maps file/folder paths to their set of corresponding watch callbacks | |
* There may be more than one per file/folder if a file/folder is shared between projects | |
*/ | |
constfileWatchCallbackTrackingMap=newMap< | |
CanonicalPath, | |
Set<ts.FileWatcherCallback> | |
>(); | |
constfolderWatchCallbackTrackingMap=newMap< | |
CanonicalPath, | |
Set<ts.FileWatcherCallback> | |
>(); |
typescript-eslint/packages/typescript-estree/src/create-program/createWatchProgram.ts
Lines 313 to 316 inb3ac5f6
watchCompilerHost.watchFile=saveWatchCallback(fileWatchCallbackTrackingMap); | |
watchCompilerHost.watchDirectory=saveWatchCallback( | |
folderWatchCallbackTrackingMap, | |
); |
typescript-eslint/packages/typescript-estree/src/create-program/createWatchProgram.ts
Lines 62 to 86 inb3ac5f6
functionsaveWatchCallback( | |
trackingMap:Map<string,Set<ts.FileWatcherCallback>>, | |
){ | |
return( | |
fileName:string, | |
callback:ts.FileWatcherCallback, | |
):ts.FileWatcher=>{ | |
constnormalizedFileName=getCanonicalFileName(fileName); | |
constwatchers=(():Set<ts.FileWatcherCallback>=>{ | |
letwatchers=trackingMap.get(normalizedFileName); | |
if(!watchers){ | |
watchers=newSet(); | |
trackingMap.set(normalizedFileName,watchers); | |
} | |
returnwatchers; | |
})(); | |
watchers.add(callback); | |
return{ | |
close:():void=>{ | |
watchers.delete(callback); | |
}, | |
}; | |
}; | |
} |
Looking at TS's source code...
It has one place where it passes in a constant function pointer in towatchFile
:
And many places where it passes arrow functions in towatchFile
/watchDirectory
:
- https://github.com/microsoft/TypeScript/blob/6bb1f0792ba19b662ac0384ad1db824c5de43bd3/src/compiler/watchPublic.ts#L789-L798
- https://github.com/microsoft/TypeScript/blob/6bb1f0792ba19b662ac0384ad1db824c5de43bd3/src/compiler/watchPublic.ts#L901-L930
- https://github.com/microsoft/TypeScript/blob/6bb1f0792ba19b662ac0384ad1db824c5de43bd3/src/compiler/watchPublic.ts#L935-L947
- https://github.com/microsoft/TypeScript/blob/6bb1f0792ba19b662ac0384ad1db824c5de43bd3/src/compiler/watchPublic.ts#L855-L893
- https://github.com/microsoft/TypeScript/blob/6bb1f0792ba19b662ac0384ad1db824c5de43bd3/src/compiler/watchPublic.ts#L953-L990
- https://github.com/microsoft/TypeScript/blob/6bb1f0792ba19b662ac0384ad1db824c5de43bd3/src/compiler/watchPublic.ts#L329-L330
As we (and the TS program) do not have any "cleanup" step - it means we never clear these caches. We do not know when or if we can remove functions from these caches. This means we can accumulate these callbacks indefinitely.
They're just itty bitty function references - what's the problem?!? The issue is that functions have a closure which stores variables. In some of these cases the closure contains a reference to aSourceFile
. Which means that Node cannot GC theSourceFile
(or anything else in the closure for that matter) unless we free our function reference.
This is a problem asSourceFile
s aren't the lightest data structure - so unbounded accumulation of them can cause an OOM. An extreme example of this is#3533 - wherein they had a bad config file, meaning we would throw away and recreate the watch host for each file without cleaning up its watch functions.
I don't know if this actually impacts "good" project setups - but we should really profile this and figure out what sort of memory usage this causes for different usecases (big project, small project, many projects). I don't have a good solution for this either. We can't useWeakRef
because we are the sole owner of the function reference.. so we would need another mechanism to clean up.
cc@uniqueiniquity for the TS side of things