Patchwork D12430: rust-dirstate: panic if the DirstateMap counters go below 0

login
register
mail settings
Submitter phabricator
Date April 4, 2022, 3:30 p.m.
Message ID <differential-rev-PHID-DREV-igs66s7kfdgjgcyrekl6-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50775/
State New
Headers show

Comments

phabricator - April 4, 2022, 3:30 p.m.
Alphare created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  When modifying the API I hit some... interesting errors (trying to allocate
  178GB of RAM, for example) because I failed to keep the counters correctly
  updated.
  
  This counter underflow is likely to happen when code is changed around
  and can have up to eat-your-dirstate level of consequences, which is not nice.
  
  The very small runtime cost of checking these counters should really not be an
  issue and will help us uncover bugs when/if they do appear in the future.

REPOSITORY
  rHG Mercurial

BRANCH
  stable

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

AFFECTED FILES
  rust/hg-core/src/dirstate_tree/dirstate_map.rs

CHANGE DETAILS




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

Patch

diff --git a/rust/hg-core/src/dirstate_tree/dirstate_map.rs b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
--- a/rust/hg-core/src/dirstate_tree/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
@@ -831,10 +831,20 @@ 
                 )? {
                     dropped = d;
                     if dropped.had_entry {
-                        node.descendants_with_entry_count -= 1;
+                        node.descendants_with_entry_count = node
+                            .descendants_with_entry_count
+                            .checked_sub(1)
+                            .expect(
+                                "descendants_with_entry_count should be >= 0",
+                            );
                     }
                     if dropped.was_tracked {
-                        node.tracked_descendants_count -= 1;
+                        node.tracked_descendants_count = node
+                            .tracked_descendants_count
+                            .checked_sub(1)
+                            .expect(
+                                "tracked_descendants_count should be >= 0",
+                            );
                     }
 
                     // Directory caches must be invalidated when removing a
@@ -889,10 +899,16 @@ 
                 filename,
             )? {
                 if dropped.had_entry {
-                    map.nodes_with_entry_count -= 1
+                    map.nodes_with_entry_count = map
+                        .nodes_with_entry_count
+                        .checked_sub(1)
+                        .expect("nodes_with_entry_count should be >= 0");
                 }
                 if dropped.had_copy_source {
-                    map.nodes_with_copy_source_count -= 1
+                    map.nodes_with_copy_source_count = map
+                        .nodes_with_copy_source_count
+                        .checked_sub(1)
+                        .expect("nodes_with_copy_source_count should be >= 0");
                 }
             } else {
                 debug_assert!(!was_tracked);