Skip to content

Commit

Permalink
ignore: be fastidious with file handles
Browse files Browse the repository at this point in the history
This commit fixes the symlink loop checker in the parallel directory
traverser to open fewer handles at the expense of keeping handles held
open longer.

This roughly matches the corresponding change in walkdir:
BurntSushi/walkdir@5bcc5b8

Fixes #633
  • Loading branch information
BurntSushi committed Oct 22, 2017
1 parent 2a14bf2 commit 1bf9d29
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 6 deletions.
23 changes: 22 additions & 1 deletion ignore/src/dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use std::sync::{Arc, RwLock};
use gitignore::{self, Gitignore, GitignoreBuilder};
use pathutil::{is_hidden, strip_prefix};
use overrides::{self, Override};
use same_file::Handle;
use types::{self, Types};
use {Error, Match, PartialErrorBuilder};

Expand Down Expand Up @@ -95,6 +96,9 @@ struct IgnoreInner {
compiled: Arc<RwLock<HashMap<OsString, Ignore>>>,
/// The path to the directory that this matcher was built from.
dir: PathBuf,
/// An open handle to the directory, for checking symlink loops in the
/// parallel iterator.
handle: Arc<Option<Handle>>,
/// An override matcher (default is empty).
overrides: Arc<Override>,
/// A file type matcher.
Expand Down Expand Up @@ -127,11 +131,15 @@ struct IgnoreInner {

impl Ignore {
/// Return the directory path of this matcher.
#[allow(dead_code)]
pub fn path(&self) -> &Path {
&self.0.dir
}

/// Return a handle to the directory of this matcher.
pub fn handle(&self) -> Option<&Handle> {
(*self.0.handle).as_ref()
}

/// Return true if this matcher has no parent.
pub fn is_root(&self) -> bool {
self.0.parent.is_none()
Expand Down Expand Up @@ -238,9 +246,17 @@ impl Ignore {
errs.maybe_push(err);
m
};
let handle = match Handle::from_path(dir) {
Ok(handle) => Some(handle),
Err(err) => {
errs.push(Error::from(err).with_path(dir));
None
}
};
let ig = IgnoreInner {
compiled: self.0.compiled.clone(),
dir: dir.to_path_buf(),
handle: Arc::new(handle),
overrides: self.0.overrides.clone(),
types: self.0.types.clone(),
parent: Some(self.clone()),
Expand Down Expand Up @@ -451,9 +467,14 @@ impl IgnoreBuilder {
}
gi
};
let handle = match Handle::from_path(&self.dir) {
Ok(handle) => Some(handle),
Err(_) => None,
};
Ignore(Arc::new(IgnoreInner {
compiled: Arc::new(RwLock::new(HashMap::new())),
dir: self.dir.clone(),
handle: Arc::new(handle),
overrides: self.overrides.clone(),
types: self.types.clone(),
parent: None,
Expand Down
10 changes: 5 additions & 5 deletions ignore/src/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::time::Duration;
use std::vec;

use crossbeam::sync::MsQueue;
use same_file::is_same_file;
use same_file::Handle;
use walkdir::{self, WalkDir};

use dir::{Ignore, IgnoreBuilder};
Expand Down Expand Up @@ -1308,11 +1308,11 @@ fn check_symlink_loop(
child_path: &Path,
child_depth: usize,
) -> Result<(), Error> {
let hchild = Handle::from_path(child_path).map_err(|err| {
Error::from(err).with_path(child_path).with_depth(child_depth)
})?;
for ig in ig_parent.parents().take_while(|ig| !ig.is_absolute_parent()) {
let same = try!(is_same_file(ig.path(), child_path).map_err(|err| {
Error::from(err).with_path(child_path).with_depth(child_depth)
}));
if same {
if ig.handle().map_or(true, |parent| parent == &hchild) {
return Err(Error::Loop {
ancestor: ig.path().to_path_buf(),
child: child_path.to_path_buf(),
Expand Down

0 comments on commit 1bf9d29

Please sign in to comment.