Archiver: fix race condition, dead code path, and remove spin-wait#574
Archiver: fix race condition, dead code path, and remove spin-wait#574shramanb113 wants to merge 3 commits intointernetarchive:mainfrom
Conversation
|
Hi @yzqzss , The other changes I made were just adding comments here and there. I did this because using time.sleep to run the server isn't always the approach. I promise you I put in effort to make these changes. I think they will make the codebase more robust. If you need information, about how I did it I'd be happy to explain. |
NGTmeaty
left a comment
NGTmeaty
left a comment
There was a problem hiding this comment.
These fixes look good, but I did have one question:
Thanks!
| if node == nil { | ||
| return | ||
| } | ||
| nodes = append(nodes, node) |
There was a problem hiding this comment.
This appears to be from your other closed PR. Could we get this removed?
There was a problem hiding this comment.
re-enter dead lock:
lock() -> unlock() -> lock() -> (continue) -> lock(!)
There was a problem hiding this comment.
I think this is not the correct fix?
3 participants
Summary
This PR improves the reliability and scalability of the archiver by removing
fragile timing assumptions, eliminating dead code execution, and replacing
a spin-wait in the rate limiter.
Changes
Replaces a
time.Sleepused to avoid a known race condition betweenrouter.Run()andpage.Navigate()with a deterministic synchronizationmechanism.
When asset capture and crawling are disabled, the response body was fully
discarded but processing continued, leading to MIME detection and buffering
on an already-consumed stream. An early return is added to avoid unnecessary
work and ensure correct behavior.
Replaces a busy-wait loop that periodically re-acquired a mutex with a
time-based wait calculated from the token refill rate.