Skip to content

Commit f9e0a23

Browse files
ElliottjPiercebeicause
authored andcommitted
Fix spawn batch bug (bevyengine#22672)
# Objective bevyengine#18670 introduced a small but fatal issue with batch spawning. Previous tests did not catch this bug because they tested `alloc` and `alloc_many` separately. ## Solution - Fix an off by one math mistake in `FreeBufferIterator::next`. - Fix the part where you need to `ptr.add(index)` if you want the element at `index`. Whoops. - Add a test to catch these issues next time ## Testing - CI - One new test - This was originally found [here](https://discord.com/channels/691052431525675048/749335865876021248/1464359124950319114), and the reproduction crashes on main but is fine on this branch.
1 parent 9382766 commit f9e0a23

File tree

1 file changed

+33
-2
lines changed

1 file changed

+33
-2
lines changed

crates/bevy_ecs/src/entity/remote_allocator.rs

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ impl Chunk {
139139
let head = self.first.load(Ordering::Relaxed);
140140

141141
// SAFETY: Caller ensures we are init, so the chunk was allocated via a `Vec` and the index is within the capacity.
142-
unsafe { core::slice::from_raw_parts(head, len) }
142+
unsafe { core::slice::from_raw_parts(head.add(index as usize), len) }
143143
}
144144

145145
/// Sets this entity at this index.
@@ -339,7 +339,10 @@ impl<'a> Iterator for FreeBufferIterator<'a> {
339339
}
340340

341341
let still_need = self.future_buffer_indices.len() as u32;
342-
let next_index = self.future_buffer_indices.next()?;
342+
if still_need == 0 {
343+
return None;
344+
}
345+
let next_index = self.future_buffer_indices.start;
343346
let (chunk, index, chunk_capacity) = self.buffer.index_in_chunk(next_index);
344347

345348
// SAFETY: Assured by `FreeBuffer::iter`
@@ -1115,4 +1118,32 @@ mod tests {
11151118
entities.dedup();
11161119
assert_eq!(pre_len, entities.len());
11171120
}
1121+
1122+
/// Bevy's allocator doesn't make guarantees about what order entities will be allocated in.
1123+
/// This test just exists to make sure allocations don't step on each other's toes.
1124+
#[test]
1125+
fn allocation_order_correctness() {
1126+
let mut allocator = Allocator::new();
1127+
let e0 = allocator.alloc();
1128+
let e1 = allocator.alloc();
1129+
let e2 = allocator.alloc();
1130+
let e3 = allocator.alloc();
1131+
allocator.free(e0);
1132+
allocator.free(e1);
1133+
allocator.free(e2);
1134+
allocator.free(e3);
1135+
1136+
let r0 = allocator.alloc();
1137+
let mut many = allocator.alloc_many(2);
1138+
let r1 = many.next().unwrap();
1139+
let r2 = many.next().unwrap();
1140+
assert!(many.next().is_none());
1141+
drop(many);
1142+
let r3 = allocator.alloc();
1143+
1144+
assert_eq!(r0, e3);
1145+
assert_eq!(r1, e1);
1146+
assert_eq!(r2, e2);
1147+
assert_eq!(r3, e0);
1148+
}
11181149
}

0 commit comments

Comments
 (0)