Selaa lähdekoodia

fix: excessive open file descriptors

previously, recursive directory traversal led to OS errors due to too
many simultaneously open file handles.

This PR fixes that by making sure we don't read dir until we actually
need to.
Samuel Onoja 11 kuukautta sitten
vanhempi
sitoutus
6221a63b99
5 muutettua tiedostoa jossa 64 lisäystä ja 30 poistoa
  1. 26 0
      src/fs/dir.rs
  2. 10 2
      src/fs/file.rs
  3. 25 25
      src/main.rs
  4. 1 1
      src/output/color_scale.rs
  5. 2 2
      src/output/details.rs

+ 26 - 0
src/fs/dir.rs

@@ -31,6 +31,32 @@ pub struct Dir {
 }
 }
 
 
 impl Dir {
 impl Dir {
+    /// Create a new, empty `Dir` object representing the directory at the given path.
+    ///
+    /// This function does not attempt to read the contents of the directory; it merely
+    /// initializes an instance of `Dir` with an empty `DirEntry` list and the specified path.
+    /// To populate the `Dir` object with actual directory contents, use the `read`.
+    pub fn new(path: PathBuf) -> Self {
+        Self {
+            contents: vec![],
+            path,
+        }
+    }
+
+    /// Reads the contents of the directory into DirEntry.
+    ///
+    /// It is recommended to use this method in conjunction with `new` in recursive
+    /// calls, rather than `read_dir`, to avoid holding multiple open file descriptors
+    /// simultaneously, which can lead to "too many open files" errors.
+    pub fn read(&mut self) -> io::Result<&Self> {
+        info!("Reading directory {:?}", &self.path);
+
+        self.contents = fs::read_dir(&self.path)?.collect::<Result<Vec<_>, _>>()?;
+
+        info!("Read directory success {:?}", &self.path);
+        Ok(self)
+    }
+
     /// Create a new Dir object filled with all the files in the directory
     /// Create a new Dir object filled with all the files in the directory
     /// pointed to by the given path. Fails if the directory can’t be read, or
     /// pointed to by the given path. Fails if the directory can’t be read, or
     /// isn’t actually a directory, or if there’s an IO error that occurs at
     /// isn’t actually a directory, or if there’s an IO error that occurs at

+ 10 - 2
src/fs/file.rs

@@ -313,14 +313,22 @@ impl<'dir> File<'dir> {
         false
         false
     }
     }
 
 
+    /// Initializes a new `Dir` object using the `PathBuf` of
+    /// the current file. It does not perform any validation to check if the
+    /// file is actually a directory. To verify that, use `is_directory()`.
+    pub fn to_dir(&self) -> Dir {
+        trace!("read_dir: initializating dir form path");
+        Dir::new(self.path.clone())
+    }
+
     /// If this file is a directory on the filesystem, then clone its
     /// If this file is a directory on the filesystem, then clone its
     /// `PathBuf` for use in one of our own `Dir` values, and read a list of
     /// `PathBuf` for use in one of our own `Dir` values, and read a list of
     /// its contents.
     /// its contents.
     ///
     ///
     /// Returns an IO error upon failure, but this shouldn’t be used to check
     /// Returns an IO error upon failure, but this shouldn’t be used to check
     /// if a `File` is a directory or not! For that, just use `is_directory()`.
     /// if a `File` is a directory or not! For that, just use `is_directory()`.
-    pub fn to_dir(&self) -> io::Result<Dir> {
-        trace!("to_dir: reading dir");
+    pub fn read_dir(&self) -> io::Result<Dir> {
+        trace!("read_dir: reading dir");
         Dir::read_dir(self.path.clone())
         Dir::read_dir(self.path.clone())
     }
     }
 
 

+ 25 - 25
src/main.rs

@@ -288,15 +288,8 @@ impl Exa<'_> {
             if f.points_to_directory()
             if f.points_to_directory()
                 && (self.default_input_path || !self.options.dir_action.treat_dirs_as_files())
                 && (self.default_input_path || !self.options.dir_action.treat_dirs_as_files())
             {
             {
-                trace!("matching on to_dir");
-                match f.to_dir() {
-                    Ok(d) => dirs.push(d),
-                    Err(e) if e.kind() == ErrorKind::PermissionDenied => {
-                        eprintln!("{file_path:?}: {e}");
-                        exit(exits::PERMISSION_DENIED);
-                    }
-                    Err(e) => writeln!(io::stderr(), "{file_path:?}: {e}")?,
-                }
+                trace!("matching on new Dir");
+                dirs.push(f.to_dir());
             } else {
             } else {
                 files.push(f);
                 files.push(f);
             }
             }
@@ -326,7 +319,18 @@ impl Exa<'_> {
             file_style: file_name::Options { quote_style, .. },
             file_style: file_name::Options { quote_style, .. },
             ..
             ..
         } = self.options.view;
         } = self.options.view;
-        for dir in dir_files {
+        for mut dir in dir_files {
+            let dir = match dir.read() {
+                Ok(dir) => dir,
+                Err(e) => {
+                    if e.kind() == ErrorKind::PermissionDenied {
+                        exit(exits::PERMISSION_DENIED);
+                    };
+                    writeln!(io::stderr(), "{}: {}", dir.path.display(), e)?;
+                    continue;
+                }
+            };
+
             // Put a gap between directories, or between the list of files and
             // Put a gap between directories, or between the list of files and
             // the first directory.
             // the first directory.
             if first {
             if first {
@@ -373,21 +377,17 @@ impl Exa<'_> {
                     + 1;
                     + 1;
                 let follow_links = self.options.view.follow_links;
                 let follow_links = self.options.view.follow_links;
                 if !recurse_opts.tree && !recurse_opts.is_too_deep(depth) {
                 if !recurse_opts.tree && !recurse_opts.is_too_deep(depth) {
-                    let mut child_dirs = Vec::new();
-                    for child_dir in children.iter().filter(|f| {
-                        (if follow_links {
-                            f.points_to_directory()
-                        } else {
-                            f.is_directory()
-                        }) && !f.is_all_all
-                    }) {
-                        match child_dir.to_dir() {
-                            Ok(d) => child_dirs.push(d),
-                            Err(e) => {
-                                writeln!(io::stderr(), "{}: {}", child_dir.path.display(), e)?;
-                            }
-                        }
-                    }
+                    let child_dirs = children
+                        .iter()
+                        .filter(|f| {
+                            (if follow_links {
+                                f.points_to_directory()
+                            } else {
+                                f.is_directory()
+                            }) && !f.is_all_all
+                        })
+                        .map(|f| f.to_dir())
+                        .collect::<Vec<Dir>>();
 
 
                     self.print_files(Some(&dir), children)?;
                     self.print_files(Some(&dir), children)?;
                     match self.print_dirs(child_dirs, false, false, exit_status) {
                     match self.print_dirs(child_dirs, false, false, exit_status) {

+ 1 - 1
src/output/color_scale.rs

@@ -167,7 +167,7 @@ fn update_information_recursively(
             && file.name != "."
             && file.name != "."
             && file.name != ".."
             && file.name != ".."
         {
         {
-            match file.to_dir() {
+            match file.read_dir() {
                 Ok(dir) => {
                 Ok(dir) => {
                     let files: Vec<File<'_>> = dir
                     let files: Vec<File<'_>> = dir
                         .files(dot_filter, git, git_ignoring, false, false)
                         .files(dot_filter, git, git_ignoring, false, false)

+ 2 - 2
src/output/details.rs

@@ -303,8 +303,8 @@ impl<'a> Render<'a> {
                     }) && r.tree
                     }) && r.tree
                         && !r.is_too_deep(depth.0)
                         && !r.is_too_deep(depth.0)
                     {
                     {
-                        trace!("matching on to_dir");
-                        match file.to_dir() {
+                        trace!("matching on read_dir");
+                        match file.read_dir() {
                             Ok(d) => {
                             Ok(d) => {
                                 dir = Some(d);
                                 dir = Some(d);
                             }
                             }