Skip to content

Commit 338a9e5

Browse files
fe9lixBurntSushi
authored andcommitted
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
1 parent 67dd809 commit 338a9e5

File tree

2 files changed

+18
-7
lines changed

2 files changed

+18
-7
lines changed

CHANGELOG.md

+5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22
============
33
This is a minor release with a few small new features and bug fixes.
44

5+
Bug fixes:
6+
7+
* [BUG #2664](https://github.com/BurntSushi/ripgrep/issues/2690):
8+
Fix unbounded memory growth in the `ignore` crate.
9+
510
Feature enhancements:
611

712
* [FEATURE #2684](https://github.com/BurntSushi/ripgrep/issues/2684):

crates/ignore/src/dir.rs

+13-7
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use std::{
1919
fs::{File, FileType},
2020
io::{self, BufRead},
2121
path::{Path, PathBuf},
22-
sync::{Arc, RwLock},
22+
sync::{Arc, RwLock, Weak},
2323
};
2424

2525
use crate::{
@@ -101,7 +101,7 @@ struct IgnoreInner {
101101
/// Note that this is never used during matching, only when adding new
102102
/// parent directory matchers. This avoids needing to rebuild glob sets for
103103
/// parent directories if many paths are being searched.
104-
compiled: Arc<RwLock<HashMap<OsString, Ignore>>>,
104+
compiled: Arc<RwLock<HashMap<OsString, Weak<IgnoreInner>>>>,
105105
/// The path to the directory that this matcher was built from.
106106
dir: PathBuf,
107107
/// An override matcher (default is empty).
@@ -200,9 +200,11 @@ impl Ignore {
200200
let mut ig = self.clone();
201201
for parent in parents.into_iter().rev() {
202202
let mut compiled = self.0.compiled.write().unwrap();
203-
if let Some(prebuilt) = compiled.get(parent.as_os_str()) {
204-
ig = prebuilt.clone();
205-
continue;
203+
if let Some(weak) = compiled.get(parent.as_os_str()) {
204+
if let Some(prebuilt) = weak.upgrade() {
205+
ig = Ignore(prebuilt);
206+
continue;
207+
}
206208
}
207209
let (mut igtmp, err) = ig.add_child_path(parent);
208210
errs.maybe_push(err);
@@ -214,8 +216,12 @@ impl Ignore {
214216
} else {
215217
false
216218
};
217-
ig = Ignore(Arc::new(igtmp));
218-
compiled.insert(parent.as_os_str().to_os_string(), ig.clone());
219+
let ig_arc = Arc::new(igtmp);
220+
ig = Ignore(ig_arc.clone());
221+
compiled.insert(
222+
parent.as_os_str().to_os_string(),
223+
Arc::downgrade(&ig_arc),
224+
);
219225
}
220226
(ig, errs.into_error_option())
221227
}

0 commit comments

Comments
 (0)