Cleanup old file/folder watch callbacks

@uniqueiniquity

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 in b3ac5f6

    
           /** 
        
            * 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 
        
            */ 
        
           const fileWatchCallbackTrackingMap = new Map< 
        
             CanonicalPath, 
        
             Set<ts.FileWatcherCallback> 
        
           >(); 
        
           const folderWatchCallbackTrackingMap = new Map< 
        
             CanonicalPath, 
        
             Set<ts.FileWatcherCallback> 
        
           >();

typescript-eslint/packages/typescript-estree/src/create-program/createWatchProgram.ts

Lines 313 to 316 in b3ac5f6

    
           watchCompilerHost.watchFile = saveWatchCallback(fileWatchCallbackTrackingMap); 
        
           watchCompilerHost.watchDirectory = saveWatchCallback( 
        
             folderWatchCallbackTrackingMap, 
        
           );

typescript-eslint/packages/typescript-estree/src/create-program/createWatchProgram.ts

Lines 62 to 86 in b3ac5f6

    
           function saveWatchCallback( 
        
             trackingMap: Map<string, Set<ts.FileWatcherCallback>>, 
        
           ) { 
        
             return ( 
        
               fileName: string, 
        
               callback: ts.FileWatcherCallback, 
        
             ): ts.FileWatcher => { 
        
               const normalizedFileName = getCanonicalFileName(fileName); 
        
               const watchers = ((): Set<ts.FileWatcherCallback> => { 
        
                 let watchers = trackingMap.get(normalizedFileName); 
        
                 if (!watchers) { 
        
                   watchers = new Set(); 
        
                   trackingMap.set(normalizedFileName, watchers); 
        
                 } 
        
                 return watchers; 
        
               })(); 
        
               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 to watchFile:

https://github.com/microsoft/TypeScript/blob/6bb1f0792ba19b662ac0384ad1db824c5de43bd3/src/compiler/watchPublic.ts#L312

And many places where it passes arrow functions in to watchFile/watchDirectory:

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 a SourceFile. Which means that Node cannot GC the SourceFile (or anything else in the closure for that matter) unless we free our function reference.

This is a problem as SourceFiles 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 use WeakRef 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	/**
	* 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
	*/
	const fileWatchCallbackTrackingMap = new Map<
	CanonicalPath,
	Set<ts.FileWatcherCallback>
	>();
	const folderWatchCallbackTrackingMap = new Map<
	CanonicalPath,
	Set<ts.FileWatcherCallback>
	>();

	watchCompilerHost.watchFile = saveWatchCallback(fileWatchCallbackTrackingMap);
	watchCompilerHost.watchDirectory = saveWatchCallback(
	folderWatchCallbackTrackingMap,
	);

	function saveWatchCallback(
	trackingMap: Map<string, Set<ts.FileWatcherCallback>>,
	) {
	return (
	fileName: string,
	callback: ts.FileWatcherCallback,
	): ts.FileWatcher => {
	const normalizedFileName = getCanonicalFileName(fileName);
	const watchers = ((): Set<ts.FileWatcherCallback> => {
	let watchers = trackingMap.get(normalizedFileName);
	if (!watchers) {
	watchers = new Set();
	trackingMap.set(normalizedFileName, watchers);
	}
	return watchers;
	})();
	watchers.add(callback);

	return {
	close: (): void => {
	watchers.delete(callback);
	},
	};
	};
	}

Uh oh!

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions