Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion autumn/src/security/rate_limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ impl MemoryStore {
.saturating_duration_since(bucket.last_refill)
.as_secs_f64();
bucket.tokens = elapsed.mul_add(refill_per_sec, bucket.tokens).min(burst);
bucket.last_refill = now;
bucket.last_refill = std::cmp::max(bucket.last_refill, now);

let result = if bucket.tokens >= 1.0 {
bucket.tokens -= 1.0;
Expand Down
25 changes: 25 additions & 0 deletions autumn/tests/integration/chaos_channels_snapshot_loom.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#![cfg(feature = "ws")]
use autumn_web::channels::Channels;
use loom::thread;

#[test]
fn channels_concurrent_snapshot_and_publish() {
loom::model(|| {
let channels = Channels::new(32);
let tx1 = channels.sender("test_gc");
let tx2 = tx1.clone();

let c1 = channels.clone();
let t1 = thread::spawn(move || {
let snap = c1.snapshot();
let _ = snap.get("test_gc").map(|s| s.subscriber_count);
});

let t2 = thread::spawn(move || {
tx2.send("hello").ok();
});

let _ = t1.join().unwrap();
t2.join().unwrap();
});
}
1 change: 1 addition & 0 deletions autumn/tests/integration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ mod chaos_channels_concurrent_loom;
mod chaos_channels_loom;
#[cfg(feature = "ws")]
mod chaos_channels_proptest;
mod chaos_channels_snapshot_loom;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To maintain consistency with the other chaos_channels modules in this file, please gate chaos_channels_snapshot_loom with #[cfg(feature = "ws")]. Although the file itself has an inner attribute #![cfg(feature = "ws")], conditionally compiling the module at the declaration level avoids compiling an empty module when the ws feature is disabled.

Suggested change
mod chaos_channels_snapshot_loom;
#[cfg(feature = "ws")]
mod chaos_channels_snapshot_loom;

#[cfg(feature = "ws")]
mod chaos_channels_subscribe_loom;
mod chaos_job_client_loom;
Expand Down
77 changes: 77 additions & 0 deletions autumn/tests/loom_models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,83 @@ fn rate_limit_bucket_single_winner() {
});
}

/// Models the `MemoryStore::decide` rate limit bucket time-warp race condition.
///
/// In `MemoryStore::decide`, `now = Instant::now()` is called OUTSIDE the bucket
/// lock. Thread A can capture a very early `now` and stall. Thread B captures a later
/// `now`, runs `decide`, consumes tokens, and updates `last_refill` to its later `now`.
/// Thread A then wakes up, locks the bucket, and applies its EARLY `now`.
///
/// `bucket.tokens = elapsed.mul_add(refill_per_sec, bucket.tokens).min(burst);`
/// (elapsed is 0 because `saturating_duration_since` on a future `last_refill` is 0)
///
/// Then thread A does `bucket.last_refill = now` (the EARLY `now`).
/// This clobbers thread B's later `last_refill` with an older timestamp.
/// The next request will observe an artificially long `elapsed` duration because the
/// `last_refill` was time-warped backwards, granting full tokens immediately and breaking
/// the rate limit envelope.
#[test]
fn rate_limit_time_warp() {
let show_bug = std::env::var_os("RATE_LIMIT_LOOM_SHOW_BUG").is_some();
loom::model(move || {
// Models `Instant` using an integer
let bucket_tokens = Arc::new(Mutex::new(0.0));
let bucket_last_refill = Arc::new(Mutex::new(0));

let t1 = {
let tokens = bucket_tokens.clone();
let last_refill = bucket_last_refill.clone();
thread::spawn(move || {
let now = 10;
let mut guard_t = tokens.lock().unwrap();
let mut guard_l = last_refill.lock().unwrap();

// simulate elapsed logic
let elapsed = if now > *guard_l { now - *guard_l } else { 0 };
*guard_t = (*guard_t + elapsed as f64).min(5.0);

if show_bug {
// BUGGY: update last_refill to our captured `now` regardless of what it was
*guard_l = now;
} else {
// FIXED: update last_refill to `now` ONLY IF `now` is >= current last_refill
*guard_l = std::cmp::max(*guard_l, now);
}
})
};

let t2 = {
let tokens = bucket_tokens.clone();
let last_refill = bucket_last_refill.clone();
thread::spawn(move || {
let now = 20; // happens later
let mut guard_t = tokens.lock().unwrap();
let mut guard_l = last_refill.lock().unwrap();

let elapsed = if now > *guard_l { now - *guard_l } else { 0 };
*guard_t = (*guard_t + elapsed as f64).min(5.0);

if show_bug {
*guard_l = now;
} else {
*guard_l = std::cmp::max(*guard_l, now);
}
})
};

t1.join().unwrap();
t2.join().unwrap();

// The time should not go backwards. Thread 2 happened at T=20, so the max observed time
// must be retained to prevent time warping backwards to T=10 if Thread 1 won the lock last.
assert_eq!(
*bucket_last_refill.lock().unwrap(),
20,
"last_refill must not travel backwards"
);
});
}

/// Models the exactly-once `requests_active` decrement across the
/// `MetricsFuture` completion race.
///
Expand Down
Loading