Using Find References on Linux causes Editor Services to crash with "too many open files"

[daviwil]

Seems we're hitting OS limits for file handles while walking the project's folder structure.

Relevant stack trace:

   System.AggregateException: One or more errors occurred. (Too many open files) ---> System.IO.IOException: Too many open files
       at System.IO.UnixFileSystem.FileSystemEnumerable`1.OpenDirectory(String fullPath)
       at System.IO.UnixFileSystem.FileSystemEnumerable`1.Enumerate()
       at System.IO.UnixFileSystem.FileSystemEnumerable`1..ctor(String userPath, String searchPattern, SearchOption searchOption, SearchTarget searchTarget, Func`3 translateResult)
       at System.IO.UnixFileSystem.EnumeratePaths(String path, String searchPattern, SearchOption searchOption, SearchTarget searchTarget)
       at System.IO.Directory.EnumerateFileSystemNames(String path, String searchPattern, SearchOption searchOption, Boolean includeFiles, Boolean includeDirs)
       at System.IO.Directory.EnumerateFiles(String path, String searchPattern)
       at Microsoft.PowerShell.EditorServices.Workspace.RecursivelyEnumerateFiles(String folderPath)
       at Microsoft.PowerShell.EditorServices.Workspace.RecursivelyEnumerateFiles(String folderPath)
       at Microsoft.PowerShell.EditorServices.Workspace.RecursivelyEnumerateFiles(String folderPath)
       at Microsoft.PowerShell.EditorServices.Workspace.RecursivelyEnumerateFiles(String folderPath)
       at Microsoft.PowerShell.EditorServices.Workspace.RecursivelyEnumerateFiles(String folderPath)
       at Microsoft.PowerShell.EditorServices.Workspace.RecursivelyEnumerateFiles(String folderPath)
       at Microsoft.PowerShell.EditorServices.Workspace.RecursivelyEnumerateFiles(String folderPath)
       at Microsoft.PowerShell.EditorServices.LanguageService.<FindReferencesOfSymbol>d__18.MoveNext()

Related issues:

• vscode-powershell: Workspace file scanning will throw exceptions on Linux when walking large file trees vscode-powershell#908
• ide-powershell: Using Find References on Linux causes Editor Services to crash with "too many open files" daviwil/ide-powershell#1

[TylerLeonhardt]

This + a node_modules folder makes PSES cry every time :(

[rkeithhill]

Seems like any sort of "workspace level" processing needs to be accompanied by a setting like powershell.workspace.files.exclude setting that is preset with the node_modules and .git folders.

[daviwil]

Yep, definitely. Maybe a 'foldersToExclude' configuration that gets passed along to PSES would be useful to reduce the weight of commonly large folders. It'd be nice if we could use file pattern globbing for more specific matching. @rkeithhill do you know of any APIs we could borrow from PowerShell to do file/path matching?

[rkeithhill]

$ExecutionContext.SessionState.Path.GetResolvedPSPathFromPSPath() and friends. Well that's for wildcard expansion and it doesn't really support ** recursion. :-(

[TylerLeonhardt]

From what I can tell, it looks like the issue happens here:

https://github.com/PowerShell/PowerShellEditorServices/blob/master/src/PowerShellEditorServices/Workspace/Workspace.cs#L277

This looks like it's an issue with .NET Core not cleaning up after itself since all we want are files with var patterns = new string[] { @"*.ps1", @"*.psm1", @"*.psd1" };

Directory.EnumerateFiles really shouldn't throw this error if we provide a pattern.

Also, the call to Directory.EnumerateDirectories(folderPath); also throws the error since we're recursing down the file tree.

[TylerLeonhardt]

Wait I might have figured it out...

The Directory.EnumerateFiles function has a 3rd overload where you pass in a SearchOption which can be SearchOption.AllDirectories which recurses down. This makes the function a matter of calling:

Directory.EnumerateFiles(
    folderPath,
    pattern,
    SearchOption.AllDirectories));

This call doesn't crash in a folder that crashed previously!

[TylerLeonhardt]

The full function basically looks like this:

private IEnumerable<string> RecursivelyEnumerateFiles(string folderPath)
        {
            var foundFiles = Enumerable.Empty<string>();
            var patterns = new string[] { @"*.ps1", @"*.psm1", @"*.psd1" };

            foreach (var pattern in patterns)
            {
                try
                {
                    foundFiles =
                        foundFiles.Concat(
                            Directory.EnumerateFiles(
                                folderPath,
                                pattern,
                                SearchOption.AllDirectories));
                }
                catch (DirectoryNotFoundException e)
                {
                    this.logger.WriteException(
                        $"Could not enumerate files in the path '{folderPath}' due to it being an invalid path",
                        e);
                }
                catch (PathTooLongException e)
                {
                    this.logger.WriteException(
                        $"Could not enumerate files in the path '{folderPath}' due to the path being too long",
                        e);
                }
                catch (Exception e) when (e is SecurityException || e is UnauthorizedAccessException)
                {
                    this.logger.WriteException(
                        $"Could not enumerate files in the path '{folderPath}' due to the path not being accessible",
                        e);
                }
            }
            return foundFiles;
        }

[rkeithhill]

I recommend you also catch PathTooLongException (assuming the EnumerateFiles can throw that exception) - just in case. For these cases, I usually use C#'s exception filter mechanism so you don't have to repeat the contents of the multiple catch blocks.

[daviwil]

We were using this approach in the past, but the problem is that it stops enumerating on the first exception that it finds and there's no way to resume it:

0788a3e#diff-7500716c7e03a30040a8f1664f42be23

We broke this out into our own loop so that we can catch the exceptions more granularly without interrupting the scan.

[TylerLeonhardt]

Makes sense. So you don't think this is the right solution?

[daviwil]

Yeah, because if the first file/folder we look at throws an exception, none of the other files will be scanned, leaving the user to wonder why Find References, etc doesn't work.

[TylerLeonhardt]

I get that... but if we use SearchOptions on Linux and Mac, that at least prevents a crash.

And looking at our exceptions, how often do they happen?

• DirectoryNotFoundException - will only happen if given a bad folder path (not important)
• PathTooLongException - not sure... but does this only happen on windows?
• SecurityException, UnauthorizedAccessException - these seem rare to me... the user probably has to be working out of a protected folder or something but do people do that often?

The case I'm trying to make is that, I'd rather see no results than a crash on linux/mac.

Please correct me if I have something wrong or let me know what your thoughts are.

Also, should we consider 3rd party libs? like: https://github.com/kthompson/glob

[TylerLeonhardt]

btw globstar is on the .NET Core radar:
https://github.com/dotnet/corefx/issues/18922

[daviwil]

I think PathTooLongException only happens on Windows, but it's entirely possible to hit this when working on a project that's under a nested folder path if it has it's own deep folder tree. Since users of Windows PowerShell greatly overwhelm any users of PowerShell on Linux or Mac, I think it makes sense to guard for cases that might be Windows-only while still giving the best possible language experience.

Before switching back to the old method, I'd recommend changing the current algorithm to not use the IEnumerable-returning EnumerateDirectories/EnumerateFiles since it will cause directory handles to be kept open while delving through larger file trees. Here's the code where file enumeration happens on Unix machines:

https://github.com/dotnet/corefx/blob/bffef76f6af208e2042a2f27bc081ee908bb390b/src/System.IO.FileSystem/src/System/IO/UnixFileSystem.cs#L480

If you change those EnumerateDirectories/Files calls to GetDirectories/Files, it might take care of the problem. There's really no benefit to using the Enumerate variety since we're not streaming the results back anyway, it just seemed to be the right thing to do at the time.

[TylerLeonhardt]

@daviwil wow. Changing to Get* just works. Good call! I'll send out a PR