diff options
author | fe9lix <fe9lix@googlemail.com> | 2023-12-20 18:34:42 +0100 |
---|---|---|
committer | Andrew Gallant <jamslam@gmail.com> | 2024-01-06 12:50:42 -0500 |
commit | b9c774937fc285e6668be9823c4bf231a61fc4a8 (patch) | |
tree | fb6927939b8993ba8bf808e06a1bfb6660245f01 | |
parent | 67dd809a8097923a721305ce0632a9fb9f19cb57 (diff) |
ignore: fix reference cycle for compiled matchers
It looks like there is a reference cycle caused by the compiled
matchers (compiled HashMap holds ref to Ignore and Ignore holds ref
to HashMap). Using weak refs fixes issue #2690 in my test project.
Also confirmed via before and after when profiling the code, see the
attached screenshots in #2692.
Fixes #2690
-rw-r--r-- | CHANGELOG.md | 5 | ||||
-rw-r--r-- | crates/ignore/src/dir.rs | 20 |
2 files changed, 18 insertions, 7 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index e6d56e7f..0c30d64e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ============ This is a minor release with a few small new features and bug fixes. +Bug fixes: + +* [BUG #2664](https://github.com/BurntSushi/ripgrep/issues/2690): + Fix unbounded memory growth in the `ignore` crate. + Feature enhancements: * [FEATURE #2684](https://github.com/BurntSushi/ripgrep/issues/2684): diff --git a/crates/ignore/src/dir.rs b/crates/ignore/src/dir.rs index 7f8ff103..b302943a 100644 --- a/crates/ignore/src/dir.rs +++ b/crates/ignore/src/dir.rs @@ -19,7 +19,7 @@ use std::{ fs::{File, FileType}, io::{self, BufRead}, path::{Path, PathBuf}, - sync::{Arc, RwLock}, + sync::{Arc, RwLock, Weak}, }; use crate::{ @@ -101,7 +101,7 @@ struct IgnoreInner { /// Note that this is never used during matching, only when adding new /// parent directory matchers. This avoids needing to rebuild glob sets for /// parent directories if many paths are being searched. - compiled: Arc<RwLock<HashMap<OsString, Ignore>>>, + compiled: Arc<RwLock<HashMap<OsString, Weak<IgnoreInner>>>>, /// The path to the directory that this matcher was built from. dir: PathBuf, /// An override matcher (default is empty). @@ -200,9 +200,11 @@ impl Ignore { let mut ig = self.clone(); for parent in parents.into_iter().rev() { let mut compiled = self.0.compiled.write().unwrap(); - if let Some(prebuilt) = compiled.get(parent.as_os_str()) { - ig = prebuilt.clone(); - continue; + if let Some(weak) = compiled.get(parent.as_os_str()) { + if let Some(prebuilt) = weak.upgrade() { + ig = Ignore(prebuilt); + continue; + } } let (mut igtmp, err) = ig.add_child_path(parent); errs.maybe_push(err); @@ -214,8 +216,12 @@ impl Ignore { } else { false }; - ig = Ignore(Arc::new(igtmp)); - compiled.insert(parent.as_os_str().to_os_string(), ig.clone()); + let ig_arc = Arc::new(igtmp); + ig = Ignore(ig_arc.clone()); + compiled.insert( + parent.as_os_str().to_os_string(), + Arc::downgrade(&ig_arc), + ); } (ig, errs.into_error_option()) } |