Browse Source

Fix free_ind

ticki 8 years ago
parent
commit
66cbc42050
2 changed files with 56 additions and 19 deletions
  1. 21 0
      README.md
  2. 35 19
      src/bookkeeper.rs

+ 21 - 0
README.md

@@ -218,3 +218,24 @@ fn main() {
 ### Safe SBRK
 
 ralloc provides a `sbrk`, which can be used safely without breaking the allocator.
+
+### Logging
+
+If you enable the `log` feature, you get detailed locking of the allocator, e.g.
+
+```
+|   : BRK'ing a block of size, 80, and alignment 8.            (at bookkeeper.rs:458)
+|   : Pushing 0x5578dacb2000[0x0] and 0x5578dacb2050[0xffb8].  (at bookkeeper.rs:490)
+|x  : Freeing 0x1[0x0].                                        (at bookkeeper.rs:409)
+x|  : BRK'ing a block of size, 4, and alignment 1.             (at bookkeeper.rs:458)
+x|  : Pushing 0x5578dacc2008[0x0] and 0x5578dacc200c[0xfffd].  (at bookkeeper.rs:490)
+x|x : Reallocating 0x5578dacc2008[0x4] to size 8 with align 1. (at bookkeeper.rs:272)
+x|x : Inplace reallocating 0x5578dacc2008[0x4] to size 8.      (at bookkeeper.rs:354)
+_|x : Freeing 0x5578dacb2058[0xffb0].                          (at bookkeeper.rs:409)
+_|x : Inserting block 0x5578dacb2058[0xffb0].                  (at bookkeeper.rs:635)
+```
+
+To the left, you can see the state of the block pool. `x` denotes a non-empty
+block, `_` denotes an empty block, and `|` denotes the cursor.
+
+The `a[b]` is a syntax for block on address `a` with size `b`.

+ 35 - 19
src/bookkeeper.rs

@@ -159,8 +159,9 @@ impl Bookkeeper {
         }).next() {
             let (res, excessive) = b.split(size);
 
-            // Mark the excessive space as free.
-            self.free_ind(n, excessive);
+            // Mark the excessive space as free. Since `b` was split and we push `excessive`, not
+            // `res`, we have index `n + 1` NOT `n`.
+            self.free_ind(n + 1, excessive);
             //   ^^^^ Important note to self: Do not replace the old block, it is already replaced
             //        by the alignment block. Better let `free_ind` handle that.
 
@@ -413,24 +414,39 @@ impl Bookkeeper {
         // Assertions...
         debug_assert!(self.find(&block) == ind, "Block is not inserted at the appropriate index.");
 
-        // Try to merge left, and then right.
-        if self.pool.is_empty() || {
+        {
+            // So, we want to work around some borrowck edginess...
+            let (before, after) = self.pool.split_at_mut(ind);
             // To avoid double bound checking and other shenanigans, we declare a variable holding our
             // entry's pointer.
-            let entry = &mut self.pool[ind];
-
-            // Make some handy assertions.
-            #[cfg(feature = "debug_tools")]
-            assert!(entry != &mut block, "Double free.");
-            debug_assert!(block.is_empty() || entry <= &mut block, "Block merged in the wrong \
-                          direction.");
-
-            entry.merge_right(&mut block).is_err()
-        } || ind == 0 || self.pool[ind - 1].merge_right(&mut block).is_err() {
-            // Since merge failed, we will have to insert it in a normal manner.
-            self.insert(ind, block);
+            let entry = &mut after[0];
+
+            // Try to merge it with the block to the right.
+            if entry.merge_right(&mut block).is_ok() {
+                // The merging succeeded. We proceed to try to close in the possible gap.
+                if ind != 0 {
+                    let _ = before[ind - 1].merge_right(entry);
+                }
+
+                // TODO fuck you rustc
+                // // Check consistency.
+                // self.check();
+
+                return;
+
+            // Dammit, let's try to merge left.
+            } else if ind != 0 && before[ind - 1].merge_right(entry).is_ok() {
+                // TODO fuck you rustc
+                // // Check consistency.
+                // self.check();
+
+                return;
+            }
         }
 
+        // Well, it failed, so we insert it the old-fashioned way.
+        self.insert(ind, block);
+
         // Check consistency.
         self.check();
     }
@@ -554,7 +570,7 @@ impl Bookkeeper {
         };
 
         // Move left.
-        ind - self.pool.iter().skip(ind).rev().take_while(|x| x.is_empty()).count()
+        ind - self.pool.iter().skip(ind + 1).rev().take_while(|x| x.is_empty()).count()
     }
 
     /// Insert a block entry at some index.
@@ -631,8 +647,8 @@ impl Bookkeeper {
         if let Some((n, _)) = self.pool.iter().skip(ind).enumerate().filter(|&(_, x)| !x.is_empty()).next() {
             // Reserve capacity.
             {
-                let new_len = self.pool.len() + 1;
-                self.reserve(new_len);
+                let new_len = self.pool.len();
+                self.reserve(new_len + 1);
             }
 
             unsafe {