From patchwork Thu Mar  5 08:34:42 2020
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: D8213: rust-pathauditor: use interior mutability for use in
 multi-threaded contexts
From: phabricator <phabricator@mercurial-scm.org>
X-Patchwork-Id: 45496
Message-Id: <82f0e95c438ad8fb70b980ecab67a0bf@localhost.localdomain>
To: Phabricator <phabricator@mercurial-scm.org>
Cc: mercurial-devel@mercurial-scm.org
Date: Thu, 5 Mar 2020 08:34:42 +0000

Alphare updated this revision to Diff 20499.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8213?vs=20464&id=20499

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8213/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8213

AFFECTED FILES
  rust/hg-core/src/dirstate/status.rs
  rust/hg-core/src/utils/files.rs
  rust/hg-core/src/utils/path_auditor.rs

CHANGE DETAILS




To: Alphare, #hg-reviewers
Cc: mercurial-devel

diff --git a/rust/hg-core/src/utils/path_auditor.rs b/rust/hg-core/src/utils/path_auditor.rs
--- a/rust/hg-core/src/utils/path_auditor.rs
+++ b/rust/hg-core/src/utils/path_auditor.rs
@@ -13,13 +13,14 @@
 };
 use std::collections::HashSet;
 use std::path::{Path, PathBuf};
+use std::sync::{Mutex, RwLock};
 
 /// Ensures that a path is valid for use in the repository i.e. does not use
 /// any banned components, does not traverse a symlink, etc.
 #[derive(Debug, Default)]
 pub struct PathAuditor {
-    audited: HashSet<HgPathBuf>,
-    audited_dirs: HashSet<HgPathBuf>,
+    audited: Mutex<HashSet<HgPathBuf>>,
+    audited_dirs: RwLock<HashSet<HgPathBuf>>,
     root: PathBuf,
 }
 
@@ -31,7 +32,7 @@
         }
     }
     pub fn audit_path(
-        &mut self,
+        &self,
         path: impl AsRef<HgPath>,
     ) -> Result<(), HgPathError> {
         // TODO windows "localpath" normalization
@@ -40,7 +41,7 @@
             return Ok(());
         }
         // TODO case normalization
-        if self.audited.contains(path) {
+        if self.audited.lock().unwrap().contains(path) {
             return Ok(());
         }
         // AIX ignores "/" at end of path, others raise EISDIR.
@@ -113,14 +114,14 @@
         for index in 0..parts.len() {
             let prefix = &parts[..index + 1].join(&b'/');
             let prefix = HgPath::new(prefix);
-            if self.audited_dirs.contains(prefix) {
+            if self.audited_dirs.read().unwrap().contains(prefix) {
                 continue;
             }
             self.check_filesystem(&prefix, &path)?;
-            self.audited_dirs.insert(prefix.to_owned());
+            self.audited_dirs.write().unwrap().insert(prefix.to_owned());
         }
 
-        self.audited.insert(path.to_owned());
+        self.audited.lock().unwrap().insert(path.to_owned());
 
         Ok(())
     }
@@ -171,7 +172,7 @@
         Ok(())
     }
 
-    pub fn check(&mut self, path: impl AsRef<HgPath>) -> bool {
+    pub fn check(&self, path: impl AsRef<HgPath>) -> bool {
         self.audit_path(path).is_ok()
     }
 }
@@ -184,7 +185,7 @@
 
     #[test]
     fn test_path_auditor() {
-        let mut auditor = PathAuditor::new(get_path_from_bytes(b"/tmp"));
+        let auditor = PathAuditor::new(get_path_from_bytes(b"/tmp"));
 
         let path = HgPath::new(b".hg/00changelog.i");
         assert_eq!(
diff --git a/rust/hg-core/src/utils/files.rs b/rust/hg-core/src/utils/files.rs
--- a/rust/hg-core/src/utils/files.rs
+++ b/rust/hg-core/src/utils/files.rs
@@ -210,7 +210,7 @@
     } else {
         name.to_owned()
     };
-    let mut auditor = PathAuditor::new(&root);
+    let auditor = PathAuditor::new(&root);
     if name != root && name.starts_with(&root) {
         let name = name.strip_prefix(&root).unwrap();
         auditor.audit_path(path_to_hg_path_buf(name)?)?;
diff --git a/rust/hg-core/src/dirstate/status.rs b/rust/hg-core/src/dirstate/status.rs
--- a/rust/hg-core/src/dirstate/status.rs
+++ b/rust/hg-core/src/dirstate/status.rs
@@ -698,7 +698,7 @@
             // We walked all dirs under the roots that weren't ignored, and
             // everything that matched was stat'ed and is already in results.
             // The rest must thus be ignored or under a symlink.
-            let mut path_auditor = PathAuditor::new(root_dir);
+            let path_auditor = PathAuditor::new(root_dir);
 
             for (ref filename, entry) in to_visit {
                 // Report ignored items in the dmap as long as they are not