From bcdd93a9f81f25c4c5e69cb6495b229356ad0e3e Mon Sep 17 00:00:00 2001 From: Ellen Date: Tue, 7 Jun 2022 18:59:12 +0100 Subject: [PATCH 1/3] Fix leak in `blob_vec_drop_empty_capacity` test --- .github/workflows/ci.yml | 4 +--- crates/bevy_ecs/src/storage/blob_vec.rs | 6 +++++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 56e7b468e8b69..8a14f8e260f18 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -99,11 +99,9 @@ jobs: RUSTFLAGS: -Zrandomize-layout # https://github.com/rust-lang/miri#miri--z-flags-and-environment-variables # -Zmiri-disable-isolation is needed because our executor uses `fastrand` which accesses system time. - # -Zmiri-ignore-leaks is needed because running bevy_ecs tests finds a memory leak but its impossible - # to track down because allocids are nondeterministic. # -Zmiri-permissive-provenance disables warnings against int2ptr casts (since those are used by once_cell) # -Zmiri-disable-weak-memory-emulation works around https://github.com/bevyengine/bevy/issues/5164. - MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-ignore-leaks -Zmiri-permissive-provenance -Zmiri-disable-weak-memory-emulation + MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-permissive-provenance -Zmiri-disable-weak-memory-emulation check-compiles: runs-on: ubuntu-latest diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs index 14d584b048364..0aeeda6d82e47 100644 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ b/crates/bevy_ecs/src/storage/blob_vec.rs @@ -304,12 +304,16 @@ impl BlobVec { impl Drop for BlobVec { fn drop(&mut self) { self.clear(); + if self.item_layout.size() > 0 { + unsafe { + std::alloc::dealloc(self.swap_scratch.as_ptr(), self.item_layout); + } + } let array_layout = array_layout(&self.item_layout, self.capacity).expect("array layout should be valid"); if array_layout.size() > 0 { unsafe { std::alloc::dealloc(self.get_ptr_mut().as_ptr(), array_layout); - std::alloc::dealloc(self.swap_scratch.as_ptr(), self.item_layout); } } } From 960a7a8bfc390962497f7928030b1159792a64ac Mon Sep 17 00:00:00 2001 From: Ellen Date: Tue, 7 Jun 2022 20:39:41 +0100 Subject: [PATCH 2/3] Fix leak in `panic_while_overwriting_component` test --- crates/bevy_ecs/src/storage/blob_vec.rs | 12 ++++++++++++ crates/bevy_ecs/src/world/mod.rs | 1 + 2 files changed, 13 insertions(+) diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs index 0aeeda6d82e47..1a4404d9b2729 100644 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ b/crates/bevy_ecs/src/storage/blob_vec.rs @@ -151,8 +151,20 @@ impl BlobVec { let ptr = self.get_unchecked_mut(index).promote().as_ptr(); self.len = 0; // Drop the old value, then write back, justifying the promotion + // If the drop impl for the old value panics then we run the drop impl for `value` too. if let Some(drop) = self.drop { + struct OnDrop(F); + impl Drop for OnDrop { + fn drop(&mut self) { + (self.0)(); + } + } + let value = value.as_ptr(); + let on_unwind = OnDrop(|| (drop)(OwningPtr::new(NonNull::new_unchecked(value)))); + (drop)(OwningPtr::new(NonNull::new_unchecked(ptr))); + + core::mem::forget(on_unwind); } std::ptr::copy_nonoverlapping::(value.as_ptr(), ptr, self.item_layout.size()); self.len = old_len; diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 5f4a952e62c35..d220495a82e1f 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1693,6 +1693,7 @@ mod tests { DropLogItem::Create(0), DropLogItem::Create(1), DropLogItem::Drop(0), + DropLogItem::Drop(1), ] ); } From aeff15d615d3a2259b01b85e2a5a739d9ed2cc4a Mon Sep 17 00:00:00 2001 From: Ellen Date: Fri, 1 Jul 2022 21:58:24 +0100 Subject: [PATCH 3/3] readd ignore leaks to CI --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8a14f8e260f18..abb68fee29ee1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -101,7 +101,8 @@ jobs: # -Zmiri-disable-isolation is needed because our executor uses `fastrand` which accesses system time. # -Zmiri-permissive-provenance disables warnings against int2ptr casts (since those are used by once_cell) # -Zmiri-disable-weak-memory-emulation works around https://github.com/bevyengine/bevy/issues/5164. - MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-permissive-provenance -Zmiri-disable-weak-memory-emulation + # -Zmiri-ignore-leaks is necessary because a bunch of tests don't join all threads before finishing. + MIRIFLAGS: -Zmiri-ignore-leaks -Zmiri-disable-isolation -Zmiri-permissive-provenance -Zmiri-disable-weak-memory-emulation check-compiles: runs-on: ubuntu-latest