Fix Unbounded Memory Growth (Memory Leak) in TweenController
In
[`TweenController`](file:///home/dietrich/Projekte/Source/turtlers/turtle-lib/src/tweening.rs),
commands are currently accumulated in `queue: CommandQueue` (which wraps
a `Vec<TurtleCommand>`). When commands execute in `update()`, an
internal `cursor: usize` increments, but executed commands are never
removed. In streaming and threaded applications (such as
[`examples/clock_threaded.rs`](file:///home/dietrich/Projekte/Source/turtlers/turtle-lib/examples/clock_threaded.rs)),
new commands are appended continuously, causing `queue` to grow
unboundedly over time and exhaust memory.
We will replace `queue: CommandQueue` and `cursor: usize` with `queue:
std::collections::VecDeque<TurtleCommand>` inside `TweenController`.
Consumed commands will be removed immediately using `pop_front()`,
completely freeing memory as commands complete.
> [!NOTE]
> All design decisions were aligned during the `/grill-me` session:
> - `TweenController` will internally store
`std::collections::VecDeque<TurtleCommand>` and consume items via
`pop_front()`.
> - `self.cursor` is eliminated entirely.
> - `CommandQueue` remains the public API container for passing
batches of commands into `TweenController::new` and
`TweenController::append_commands`.
> - `TurtleCommand::Reset` retains its normal semantics (clearing
drawings and parameters without clearing future queued commands), as
continuous `pop_front()` already discards consumed commands.
> - Standard `VecDeque` capacity behavior is maintained (no
auto-shrinking) to prevent reallocation jitter in streaming scenarios.
[tweening.rs](file:///home/dietrich/Projekte/Source/turtlers/turtle-lib/src/tweening.rs)
- Import `std::collections::VecDeque`.
- Update `TweenController` struct definition:
- Replace `queue: CommandQueue` and `cursor: usize` with `queue:
VecDeque<TurtleCommand>`.
- In `TweenController::new`:
- Initialize `queue` using `queue.into_iter().collect()`.
- Remove `cursor` initialization.
- In `TweenController::append_commands`:
- Extend `self.queue` directly from `new_queue: CommandQueue`.
- In `TweenController::update`:
- **Instant mode**: Replace `while let Some(command) =
self.queue.get(self.cursor).cloned()` with `while let Some(command)
= self.queue.pop_front()`, removing `self.cursor += 1` and
unnecessary command cloning.
- **Animated mode**: Replace `if let Some(command) =
self.queue.get(self.cursor).cloned()` with `if let Some(command) =
self.queue.pop_front()`, removing `self.cursor += 1` and unnecessary
command cloning.
- In `TweenController::is_complete`:
- Update check to `self.current_tween.is_none() &&
self.queue.is_empty()`.
- Add unit tests in `#[cfg(test)] mod tests`:
- Verify queue drains to 0 length in Instant mode.
- Verify queue drains in Animated mode with multiple frames.
- Verify streaming `append_commands` calls continuously drain and do
not accumulate.
- Verify `is_complete()` transitions correctly.
---
[commands.rs](file:///home/dietrich/Projekte/Source/turtlers/turtle-lib/src/commands.rs)
- Update doc comments referencing `TweenController`'s cursor to reflect
the queue consumption model.
- Run `cargo test -p turtle-lib --lib tweening` to execute the new unit
tests.
- Run `cargo test -p turtle-lib` to ensure all existing tests in
`turtle-lib` continue to pass.
- Run `cargo check --examples` to ensure all examples (including
`clock_threaded.rs`) compile cleanly.
- Review memory behavior or run `examples/clock_threaded.rs` in debug to
confirm streaming updates run smoothly.
This commit is contained in:
@@ -60,8 +60,8 @@ pub enum TurtleCommand {
|
||||
/// A pure-data sequence of turtle commands.
|
||||
///
|
||||
/// `CommandQueue` is intentionally *not* an `Iterator` — it carries no cursor
|
||||
/// state. Execution state ("which command are we on?") belongs to the
|
||||
/// consumer; `TweenController` owns the cursor that walks this queue.
|
||||
/// state. Execution state belongs to the consumer; `TweenController` consumes
|
||||
/// this queue as commands execute.
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct CommandQueue {
|
||||
commands: Vec<TurtleCommand>,
|
||||
@@ -116,7 +116,7 @@ impl Default for CommandQueue {
|
||||
///
|
||||
/// This is used by `CommandQueue::extend` and `TweenController::append_commands`
|
||||
/// to drain one queue into another. It does *not* imply that `CommandQueue`
|
||||
/// itself is stateful; the cursor always lives in the consumer.
|
||||
/// itself is stateful; execution state is managed by the consumer.
|
||||
impl IntoIterator for CommandQueue {
|
||||
type Item = TurtleCommand;
|
||||
type IntoIter = std::vec::IntoIter<TurtleCommand>;
|
||||
|
||||
+161
-18
@@ -5,6 +5,7 @@ use crate::commands::{CommandQueue, TurtleCommand};
|
||||
use crate::general::{AnimationSpeed, Radians};
|
||||
use crate::state::{DrawCommand, FillState, TurtleParams};
|
||||
use macroquad::prelude::*;
|
||||
use std::collections::VecDeque;
|
||||
use tween::{CubicInOut, TweenValue, Tweener};
|
||||
|
||||
// Newtype wrapper for Vec2 to implement TweenValue
|
||||
@@ -46,11 +47,7 @@ impl From<TweenVec2> for Vec2 {
|
||||
/// Controls tweening of turtle commands
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub(crate) struct TweenController {
|
||||
queue: CommandQueue,
|
||||
/// Cursor into `queue` — tracks which command executes next.
|
||||
/// Lives here, not in `CommandQueue`, so that cloning or appending to the
|
||||
/// queue never silently resets or mid-stream-shifts the execution position.
|
||||
cursor: usize,
|
||||
queue: VecDeque<TurtleCommand>,
|
||||
current_tween: Option<CommandTween>,
|
||||
speed: AnimationSpeed,
|
||||
}
|
||||
@@ -74,8 +71,7 @@ impl TweenController {
|
||||
#[must_use]
|
||||
pub fn new(queue: CommandQueue, speed: AnimationSpeed) -> Self {
|
||||
Self {
|
||||
queue,
|
||||
cursor: 0,
|
||||
queue: queue.into_iter().collect(),
|
||||
current_tween: None,
|
||||
speed,
|
||||
}
|
||||
@@ -87,8 +83,8 @@ impl TweenController {
|
||||
|
||||
/// Append commands to the queue.
|
||||
///
|
||||
/// The cursor is **not** reset — commands already consumed remain consumed,
|
||||
/// and the new commands are picked up naturally as the cursor advances.
|
||||
/// Consumed commands are removed as they execute, and new commands
|
||||
/// are queued at the back.
|
||||
pub fn append_commands(&mut self, new_queue: CommandQueue) {
|
||||
self.queue.extend(new_queue);
|
||||
}
|
||||
@@ -117,9 +113,8 @@ impl TweenController {
|
||||
Vec::new();
|
||||
let mut draw_call_count = 0;
|
||||
|
||||
// Advance cursor through the queue for each command consumed
|
||||
while let Some(command) = self.queue.get(self.cursor).cloned() {
|
||||
self.cursor += 1;
|
||||
// Consume commands from the front of the queue
|
||||
while let Some(command) = self.queue.pop_front() {
|
||||
// Handle SetSpeed command to potentially switch modes
|
||||
if let TurtleCommand::SetSpeed(new_speed) = &command {
|
||||
params.speed = *new_speed;
|
||||
@@ -168,7 +163,7 @@ impl TweenController {
|
||||
|
||||
// Process current tween
|
||||
if let Some(ref mut tween) = self.current_tween {
|
||||
let elapsed = get_time() - tween.start_time;
|
||||
let elapsed = current_time() - tween.start_time;
|
||||
|
||||
// Use tweeners to calculate current values
|
||||
// For circles, calculate position along the arc instead of straight line
|
||||
@@ -273,9 +268,7 @@ impl TweenController {
|
||||
}
|
||||
|
||||
// Start next tween
|
||||
if let Some(command) = self.queue.get(self.cursor).cloned() {
|
||||
self.cursor += 1;
|
||||
|
||||
if let Some(command) = self.queue.pop_front() {
|
||||
// Handle commands that should execute immediately (no animation)
|
||||
match &command {
|
||||
TurtleCommand::SetSpeed(new_speed) => {
|
||||
@@ -325,7 +318,7 @@ impl TweenController {
|
||||
self.current_tween = Some(CommandTween {
|
||||
turtle_id,
|
||||
command,
|
||||
start_time: get_time(),
|
||||
start_time: current_time(),
|
||||
duration,
|
||||
start_params: params.clone(),
|
||||
target_params: target_state.clone(),
|
||||
@@ -342,7 +335,7 @@ impl TweenController {
|
||||
|
||||
#[must_use]
|
||||
pub fn is_complete(&self) -> bool {
|
||||
self.current_tween.is_none() && self.cursor >= self.queue.len()
|
||||
self.current_tween.is_none() && self.queue.is_empty()
|
||||
}
|
||||
|
||||
/// Get the current active tween if one is in progress
|
||||
@@ -400,3 +393,153 @@ pub(crate) fn normalize_angle(angle: f32) -> f32 {
|
||||
|
||||
normalized
|
||||
}
|
||||
|
||||
#[inline]
|
||||
fn current_time() -> f64 {
|
||||
#[cfg(test)]
|
||||
{
|
||||
use std::sync::OnceLock;
|
||||
use std::time::Instant;
|
||||
static START: OnceLock<Instant> = OnceLock::new();
|
||||
START.get_or_init(Instant::now).elapsed().as_secs_f64()
|
||||
}
|
||||
#[cfg(not(test))]
|
||||
{
|
||||
macroquad::time::get_time()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::commands::TurtleCommand;
|
||||
use crate::general::Degrees;
|
||||
use crate::state::TurtleParams;
|
||||
|
||||
fn make_test_params() -> TurtleParams {
|
||||
TurtleParams {
|
||||
position: vec2(0.0, 0.0),
|
||||
heading: 0.0,
|
||||
pen_down: true,
|
||||
pen_width: 1.0,
|
||||
color: Color::new(0.0, 0.0, 0.0, 1.0),
|
||||
fill_color: None,
|
||||
visible: true,
|
||||
shape: crate::shapes::TurtleShape::turtle(),
|
||||
speed: AnimationSpeed::Instant(100),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_instant_mode_drains_queue() {
|
||||
let mut queue = CommandQueue::new();
|
||||
queue.push(TurtleCommand::Move(100.0));
|
||||
queue.push(TurtleCommand::Turn(Degrees::new(90.0)));
|
||||
queue.push(TurtleCommand::PenUp);
|
||||
queue.push(TurtleCommand::Move(50.0));
|
||||
|
||||
let mut controller = TweenController::new(queue, AnimationSpeed::Instant(100));
|
||||
assert_eq!(controller.queue.len(), 4);
|
||||
assert!(!controller.is_complete());
|
||||
|
||||
let mut params = make_test_params();
|
||||
let mut filling = None;
|
||||
let mut commands = Vec::new();
|
||||
let mut svg_log = crate::state::SvgLog::default();
|
||||
|
||||
let completed = controller.update(0, &mut params, &mut filling, &mut commands, &mut svg_log);
|
||||
|
||||
assert_eq!(controller.queue.len(), 0, "Queue must be empty after instant update");
|
||||
assert!(controller.is_complete(), "Controller must be complete when queue is drained");
|
||||
assert!(!completed.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_streaming_append_commands_does_not_accumulate() {
|
||||
let mut controller = TweenController::new(CommandQueue::new(), AnimationSpeed::Instant(100));
|
||||
let mut params = make_test_params();
|
||||
let mut filling = None;
|
||||
let mut commands = Vec::new();
|
||||
let mut svg_log = crate::state::SvgLog::default();
|
||||
|
||||
// Simulate streaming commands across 50 frames (like clock_threaded)
|
||||
for _ in 0..50 {
|
||||
let mut batch = CommandQueue::new();
|
||||
batch.push(TurtleCommand::Reset);
|
||||
batch.push(TurtleCommand::PenDown);
|
||||
batch.push(TurtleCommand::Move(10.0));
|
||||
batch.push(TurtleCommand::Turn(Degrees::new(30.0)));
|
||||
|
||||
controller.append_commands(batch);
|
||||
assert_eq!(controller.queue.len(), 4);
|
||||
|
||||
controller.update(0, &mut params, &mut filling, &mut commands, &mut svg_log);
|
||||
|
||||
// Verify queue is pruned back to 0 — no memory leak / accumulation
|
||||
assert_eq!(controller.queue.len(), 0);
|
||||
assert!(controller.is_complete());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_instant_mode_respects_batch_limit_and_retains_pending() {
|
||||
let mut queue = CommandQueue::new();
|
||||
// 5 drawing commands
|
||||
queue.push(TurtleCommand::Move(10.0));
|
||||
queue.push(TurtleCommand::Move(20.0));
|
||||
queue.push(TurtleCommand::Move(30.0));
|
||||
queue.push(TurtleCommand::Move(40.0));
|
||||
queue.push(TurtleCommand::Move(50.0));
|
||||
|
||||
// Limit to 2 draw calls per frame
|
||||
let mut controller = TweenController::new(queue, AnimationSpeed::Instant(2));
|
||||
let mut params = make_test_params();
|
||||
let mut filling = None;
|
||||
let mut commands = Vec::new();
|
||||
let mut svg_log = crate::state::SvgLog::default();
|
||||
|
||||
// Frame 1: processes 2 drawing commands
|
||||
let completed1 = controller.update(0, &mut params, &mut filling, &mut commands, &mut svg_log);
|
||||
assert_eq!(completed1.len(), 2);
|
||||
assert_eq!(controller.queue.len(), 3, "3 commands should remain in queue");
|
||||
assert!(!controller.is_complete());
|
||||
|
||||
// Frame 2: processes next 2 drawing commands
|
||||
let completed2 = controller.update(0, &mut params, &mut filling, &mut commands, &mut svg_log);
|
||||
assert_eq!(completed2.len(), 2);
|
||||
assert_eq!(controller.queue.len(), 1, "1 command should remain in queue");
|
||||
assert!(!controller.is_complete());
|
||||
|
||||
// Frame 3: processes last drawing command
|
||||
let completed3 = controller.update(0, &mut params, &mut filling, &mut commands, &mut svg_log);
|
||||
assert_eq!(completed3.len(), 1);
|
||||
assert_eq!(controller.queue.len(), 0, "Queue must be completely drained");
|
||||
assert!(controller.is_complete());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_animated_mode_pops_to_current_tween() {
|
||||
let mut queue = CommandQueue::new();
|
||||
queue.push(TurtleCommand::Move(100.0));
|
||||
queue.push(TurtleCommand::Move(50.0));
|
||||
|
||||
let mut controller = TweenController::new(
|
||||
queue,
|
||||
AnimationSpeed::Animated(100.0),
|
||||
);
|
||||
assert_eq!(controller.queue.len(), 2);
|
||||
assert!(!controller.is_complete());
|
||||
|
||||
let mut params = make_test_params();
|
||||
let mut filling = None;
|
||||
let mut commands = Vec::new();
|
||||
let mut svg_log = crate::state::SvgLog::default();
|
||||
|
||||
// Calling update should pop the first command into current_tween
|
||||
controller.update(0, &mut params, &mut filling, &mut commands, &mut svg_log);
|
||||
|
||||
assert_eq!(controller.queue.len(), 1, "First command must be popped into current_tween");
|
||||
assert!(controller.current_tween().is_some());
|
||||
assert!(!controller.is_complete());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user