fix(tracker): add rate limiter to prevent event dispatch loops

[tusharmath]

Summary

Add rate limiter to the event tracker to prevent runaway dispatch loops that can occur when stderr/stdout are closed, causing each write error to trigger another error event.

Context

The tracker can enter a runaway loop scenario when stdout/stderr becomes unavailable (e.g., when the output stream is closed). Each write error would trigger another error event, potentially creating an infinite loop of error events. This was identified as a potential issue during error handling improvements.

Changes

• Added new RateLimiter module (rate_limit.rs) with a thread-safe, atomic-based rate limiter
• Configured maximum of 1,000 events per minute to allow normal tracking while preventing loops
• Integrated rate limiting check into the dispatch method - events exceeding the limit are silently dropped
• Added unit tests for the rate limiter to verify behavior

Key Implementation Details

The rate limiter uses atomic operations for thread-safety without requiring a mutex:

• Uses AtomicU64 for window start timestamp and AtomicUsize for event count
• Resets the window and counter every 60 seconds using compare-exchange to avoid race conditions
• Returns true if event is allowed, false if rate limit exceeded (event is dropped)

The 1,000 events/minute limit was chosen to:

• Allow normal tracking for long-running sessions
• Prevent runaway loops from overwhelming the system
• Be generous enough for legitimate high-frequency events

Use Cases

• Long-running CLI sessions with continuous event tracking
• Preventing infinite error loops when stdout/stderr becomes unavailable
• High-frequency operation monitoring without system overload

Testing

# Run the tracker tests
cargo test -p forge_tracker

# Run just the rate limiter tests
cargo test -p forge_tracker rate_limit

Links

• Related issue: (add issue number if applicable)

[graphite-app]

Critical race condition: The window reset is not atomic with the count reset. After window_start is updated via compare_exchange (line 36), but before count.store(0) executes (line 39), other threads may see the new window_start value and skip the reset check (line 31), then increment the old (un-reset) count value (line 44). This allows events beyond the rate limit.

Scenario:

1. Thread A: CAS succeeds, sets window_start to new time
2. Thread A: Gets preempted before executing count.store(0)
3. Thread B: Loads new window_start, sees no reset needed (time diff < 60)
4. Thread B: Increments old count value (e.g., 1005) via fetch_add
5. Thread A: Resumes and resets count to 0
6. Result: Events from the old window weren't properly limited

Fix: Use a single atomic operation or add a generation counter:

let current_count = self.count.fetch_add(1, Ordering::Relaxed);
if now.saturating_sub(window_start) >= 60 && current_count >= self.max_per_minute {
    if self.window_start.compare_exchange(window_start, now, Ordering::SeqCst, Ordering::Relaxed).is_ok() {
        self.count.store(1, Ordering::SeqCst);
        return true;
    }
}
current_count < self.max_per_minute

Spotted by Graphite



Is this helpful? React 👍 or 👎 to let us know.