Fixed soundness hole that was allowing multiple mutable borrows of the same node

Author: sunjay

src/map.rs

@@ -301,10 +301,10 @@ impl<K: Ord, V> BSTMap<K, V> {
               Q: Ord + ?Sized,
     {
         let mut current = self.root_mut();
-        while let Some(mut node) = current.take() {
+        while let Some(node) = current.take() {
             match key.cmp(node.key().borrow()) {
-                Ordering::Less => current = node.left(),
-                Ordering::Greater => current = node.right(),
+                Ordering::Less => current = node.into_left(),
+                Ordering::Greater => current = node.into_right(),
                 Ordering::Equal => return Some(node.into_value_mut()),
             }
         }
@@ -374,10 +374,10 @@ impl<K: Ord, V> BSTMap<K, V> {
               Q: Ord + ?Sized,
     {
         let mut current = self.root_mut();
-        while let Some(mut node) = current {
+        while let Some(node) = current {
             match key.cmp(node.key().borrow()) {
-                Ordering::Less => current = node.left(),
-                Ordering::Greater => current = node.right(),
+                Ordering::Less => current = node.into_left(),
+                Ordering::Greater => current = node.into_right(),
                 Ordering::Equal => return Some(node.into_entry_mut()),
             }
         }
@@ -422,7 +422,7 @@ impl<K: Ord, V> BSTMap<K, V> {
                         node.push_left(key, value);
                         break;
                     }
-                    current = node.left();
+                    current = node.into_left();
                 },
 
                 Ordering::Greater => {
@@ -431,7 +431,7 @@ impl<K: Ord, V> BSTMap<K, V> {
                         node.push_right(key, value);
                         break;
                     }
-                    current = node.right();
+                    current = node.into_right();
                 },
 
                 Ordering::Equal => {

src/map/node.rs

@@ -192,15 +192,31 @@ impl<'a, K, V> NodeMut<'a, K, V> {
     }
 
     /// Returns the left child node (subtree) of this node, if any
-    pub fn left(&mut self) -> Option<Self> {
+    pub fn left(&'a mut self) -> Option<Self> {
         // Safety: Nodes only contain pointers to other nodes within `self.nodes`
         //   Importantly, those pointers are to other *distinct* nodes, so there will never be any
         //   overlap (we know they are distinct because trees have no cycles by definition)
         self.node.left.map(move |ptr| unsafe { Self::new(self.nodes, ptr) })
     }
 
     /// Returns the right child node (subtree) of this node, if any
-    pub fn right(&mut self) -> Option<Self> {
+    pub fn right(&'a mut self) -> Option<Self> {
+        // Safety: Nodes only contain pointers to other nodes within `self.nodes`
+        //   Importantly, those pointers are to other *distinct* nodes, so there will never be any
+        //   overlap (we know they are distinct because trees have no cycles by definition)
+        self.node.right.map(move |ptr| unsafe { Self::new(self.nodes, ptr) })
+    }
+
+    /// Returns the left child node (subtree) of this node, if any
+    pub fn into_left(self) -> Option<Self> {
+        // Safety: Nodes only contain pointers to other nodes within `self.nodes`
+        //   Importantly, those pointers are to other *distinct* nodes, so there will never be any
+        //   overlap (we know they are distinct because trees have no cycles by definition)
+        self.node.left.map(move |ptr| unsafe { Self::new(self.nodes, ptr) })
+    }
+
+    /// Returns the right child node (subtree) of this node, if any
+    pub fn into_right(self) -> Option<Self> {
         // Safety: Nodes only contain pointers to other nodes within `self.nodes`
         //   Importantly, those pointers are to other *distinct* nodes, so there will never be any
         //   overlap (we know they are distinct because trees have no cycles by definition)