diff --git a/.github/workflows/Miri.yml b/.github/workflows/Miri.yml new file mode 100644 index 00000000..03fa7ba2 --- /dev/null +++ b/.github/workflows/Miri.yml @@ -0,0 +1,47 @@ +name: Miri tests + +on: + push: + paths-ignore: + - '.devcontainer/**' + - '.gitpod.yml' + - '.vscode/**' + - 'tests/**' + pull_request: + paths-ignore: + - '.devcontainer/**' + - '.gitpod.yml' + - '.vscode/**' + - 'tests/**' + schedule: + # Run against the last commit on the default branch on Friday at 9pm (UTC?) + - cron: '0 21 * * 5' + +jobs: + test: + runs-on: ubuntu-latest + + steps: + - name: Checkout Moka + uses: actions/checkout@v2 + + - name: Install Rust nightly toolchain with Miri + uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: nightly + override: true + components: miri + + - uses: Swatinem/rust-cache@v1 + + - name: cargo clean + uses: actions-rs/cargo@v1 + with: + command: clean + + - name: Run Miri test (deque) + uses: actions-rs/cargo@v1 + with: + command: miri + args: test deque diff --git a/src/common/deque.rs b/src/common/deque.rs index f55f0391..1bf9596c 100644 --- a/src/common/deque.rs +++ b/src/common/deque.rs @@ -246,6 +246,17 @@ impl Deque { self.len -= 1; } + /// Unlinks the specified node from the current list, and then drop the node. + /// + /// This method takes care not to create mutable references to `element`, to + /// maintain validity of aliasing pointers. + /// + /// Panics: + pub(crate) unsafe fn unlink_and_drop(&mut self, node: NonNull>) { + self.unlink(node); + std::mem::drop(Box::from_raw(node.as_ptr())); + } + #[allow(unused)] pub(crate) fn reset_cursor(&mut self) { self.cursor = None; @@ -504,11 +515,7 @@ mod tests { assert!(!deque.contains(node3_ref)); assert!(node3_ref.next.is_none()); assert!(node3_ref.next.is_none()); - - // This does not work as expected because NonNull implements Copy trait. - // See clippy(clippy::drop_copy) at - // https://rust-lang.github.io/rust-clippy/master/index.html#drop_copy - // std::mem::drop(node3_ptr); + std::mem::drop(unsafe { Box::from_raw(node3_ptr.as_ptr()) }); // peek_front() -> node2 let head_h = deque.peek_front().unwrap(); @@ -541,7 +548,7 @@ mod tests { assert!(!deque.contains(node2_ref)); assert!(node2_ref.next.is_none()); assert!(node2_ref.next.is_none()); - // std::mem::drop(node2_ptr); + std::mem::drop(unsafe { Box::from_raw(node2_ptr.as_ptr()) }); // peek_front() -> node1 let head_g = deque.peek_front().unwrap(); @@ -568,7 +575,7 @@ mod tests { assert!(!deque.contains(node1_ref)); assert!(node1_ref.next.is_none()); assert!(node1_ref.next.is_none()); - // std::mem::drop(node1_ptr); + std::mem::drop(unsafe { Box::from_raw(node1_ptr.as_ptr()) }); // peek_front() -> node1 let head_h = deque.peek_front(); @@ -629,7 +636,7 @@ mod tests { // Try to unlink during iteration. assert_eq!((&mut deque).next(), Some(&"a".into())); // Next will be "c", but we unlink it. - unsafe { deque.unlink(node3_ptr) }; + unsafe { deque.unlink_and_drop(node3_ptr) }; // Now, next should be "b". assert_eq!((&mut deque).next(), Some(&"b".into())); assert!((&mut deque).next().is_none()); @@ -691,7 +698,7 @@ mod tests { // ------------------------------------------------------- // Iterate after an unlink. // Unlink the second node "c". Now "a" -> "c". - unsafe { deque.unlink(node3_ptr) }; + unsafe { deque.unlink_and_drop(node3_ptr) }; let node1a = deque.peek_front().unwrap(); assert_eq!(node1a.element, "a".to_string()); let node2a = node1a.next_node().unwrap(); diff --git a/src/sync/deques.rs b/src/sync/deques.rs index 99e0bd13..68f400d7 100644 --- a/src/sync/deques.rs +++ b/src/sync/deques.rs @@ -166,7 +166,8 @@ impl Deques { let p = node.as_ref(); if &p.region == deq.region() { if deq.contains(p) { - deq.unlink(node); + // https://github.com/moka-rs/moka/issues/64 + deq.unlink_and_drop(node); } } else { panic!( @@ -181,7 +182,8 @@ impl Deques { let p = node.as_ref(); if &p.region == deq.region() { if deq.contains(p) { - deq.unlink(node); + // https://github.com/moka-rs/moka/issues/64 + deq.unlink_and_drop(node); } } else { panic!( diff --git a/src/unsync/deques.rs b/src/unsync/deques.rs index 56d39aee..844a74b4 100644 --- a/src/unsync/deques.rs +++ b/src/unsync/deques.rs @@ -122,7 +122,8 @@ impl Deques { node: NonNull>>, ) { if deq.contains(node.as_ref()) { - deq.unlink(node); + // https://github.com/moka-rs/moka/issues/64 + deq.unlink_and_drop(node); } else { panic!( "unlink_node - node is not a member of {} deque. {:?}", @@ -138,7 +139,8 @@ impl Deques { let p = node.as_ref(); debug_assert_eq!(&p.region, &WriteOrder); if deq.contains(p) { - deq.unlink(node); + // https://github.com/moka-rs/moka/issues/64 + deq.unlink_and_drop(node); } else { panic!( "unlink_node - node is not a member of write_order deque. {:?}",