search

anatol / pacoloco

Caching proxy server for Arch Linux pacman
MIT License
199 stars 30 forks source link

Guard urls during a mirrorlist refresh #103

Closed krameler closed 4 months ago

krameler commented 4 months ago

This PR tries to fix #92, by locking access to the Mirrorlist's Urls during refresh.

I had the same problem on my deployed pacoloco and also successfully reproduced the issue using @chennin's script. Using this PR, both were unable to reproduce the error on my system.

I'm new to go and it's concurrency model, but my guess is that the repo archlinux has no urls error happens due to a race condition in getMirrorlistURLs(): While the Mirrorlist File is read for the first time (e.g. because of a request for 'core.db') the URLs array is still empty but LastMirrorlistCheck is already set. When in this moment the function is called concurrently (e.g. because of a parallel download request for 'extra.db') it returns the still empty URLs array due to a recent LastMirrorlistCheck, instead of waiting for it to be populated.

I used a mutex instead of setting the LastMirrorlistCheck after URLs is populated to block multiple goroutines from reading and parsing the same Mirrorlist simultaneously. However this doesn't fix any possible racing with reflector or similar.

Would love for someone to try reproducing #92 with this PR on their system and review my first work with go.

anatol commented 4 months ago

The PR looks good to me. I'll test it a bit and then merge, unless there are objections.

chennin commented 4 months ago

tested and looks like it works for me. Thank you!

anatol commented 4 months ago

Thank you @krameler for your work. The PR has landed mastrer branch.