Skip to content

Commit

Permalink
Fix double drop in BlobVec::replace_unchecked (bevyengine#2597)
Browse files Browse the repository at this point in the history
  • Loading branch information
sapir committed Sep 20, 2021
1 parent 615d43b commit 4da5537
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 7 deletions.
7 changes: 5 additions & 2 deletions crates/bevy_ecs/src/storage/blob_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,11 @@ impl BlobVec {
pub unsafe fn replace_unchecked(&mut self, index: usize, value: *mut u8) {
debug_assert!(index < self.len());
let ptr = self.get_unchecked(index);
(self.drop)(ptr);
std::ptr::copy_nonoverlapping(value, ptr, self.item_layout.size());
// We need to make sure that we don't still contain the old value before
// calling `drop`. Otherwise, the collection `Drop` impl will call
// `drop` again.
std::ptr::swap_nonoverlapping(value, ptr, self.item_layout.size());
(self.drop)(value);
}

/// increases the length by one (and grows the vec if needed) with uninitialized memory and
Expand Down
13 changes: 8 additions & 5 deletions crates/bevy_ecs/src/storage/sparse_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,16 @@ impl ComponentSparseSet {
self.dense.len() == 0
}

/// Inserts the `entity` key and component `value` pair into this sparse set.
/// The caller is responsible for ensuring the value is not dropped. This collection will drop
/// the value when needed.
/// Inserts the `entity` key and component `value` pair into this sparse
/// set. This collection takes ownership of the contents of `value`, and
/// will drop the value when needed. Also, it may overwrite the contents of
/// the `value` pointer if convenient. The caller is responsible for
/// ensuring it does not drop `*value` after calling `insert`.
///
/// # Safety
/// The `value` pointer must point to a valid address that matches the `Layout`
/// inside the `ComponentInfo` given when constructing this sparse set.
/// The `value` pointer must point to a valid address that matches the
/// `Layout` inside the `ComponentInfo` given when constructing this sparse
/// set.
pub unsafe fn insert(&mut self, entity: Entity, value: *mut u8, change_tick: u32) {
if let Some(&dense_index) = self.sparse.get(entity) {
self.dense.replace_unchecked(dense_index, value);
Expand Down

0 comments on commit 4da5537

Please sign in to comment.