-
Notifications
You must be signed in to change notification settings - Fork 2
fix: Self-calling method in http.rs
#177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughFixed an infinite recursion bug in the HttpTracker interval accessor method. The method was incorrectly calling itself instead of reading the underlying AtomicUsize value directly, now corrected to use atomic load operation with Acquire ordering. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Possibly related issues
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/libtortillas/src/tracker/http.rs (1)
165-171: Consider removing duplicate inherent methods.The inherent methods
stats()andinterval()(lines 165-171) duplicate the trait implementations (lines 260-266). Since both implementations are identical, you could remove the inherent methods and rely solely on the trait implementations, reducing maintenance burden. However, if the inherent methods are intentionally provided to avoid vtable dispatch overhead when the concrete type is known, this pattern is acceptable.Also applies to: 260-266
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/libtortillas/src/tracker/http.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
🔇 Additional comments (1)
crates/libtortillas/src/tracker/http.rs (1)
264-266: Excellent fix for a critical infinite recursion bug!The method now correctly loads from the atomic field instead of recursively calling itself. The previous implementation would have caused a stack overflow on every call to this trait method. The use of
Ordering::Acquireis appropriate and pairs correctly with theOrdering::Releaseused inset_intervalat line 174.
#173
Summary by CodeRabbit