Guard urls during a mirrorlist refresh (#103)

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.

Closes #92

config.go

@@ -3,6 +3,7 @@ package main
 import (
 	"log"
 	"os"
+	"sync"
 	"time"
 
 	"github.com/gorhill/cronexpr"
@@ -19,12 +20,13 @@ const (
 )
 
 type Repo struct {
-	URL                  string    `yaml:"url"`
-	URLs                 []string  `yaml:"urls"`
-	Mirrorlist           string    `yaml:"mirrorlist"`
-	HttpProxy            string    `yaml:"http_proxy"`
-	LastMirrorlistCheck  time.Time `yaml:"-"`
-	LastModificationTime time.Time `yaml:"-"`
+	URL                  string     `yaml:"url"`
+	URLs                 []string   `yaml:"urls"`
+	Mirrorlist           string     `yaml:"mirrorlist"`
+	HttpProxy            string     `yaml:"http_proxy"`
+	LastMirrorlistCheck  time.Time  `yaml:"-"`
+	MirrorlistMutex      sync.Mutex `yaml:"-"`
+	LastModificationTime time.Time  `yaml:"-"`
 }
 
 type RefreshPeriod struct {

downloader.go

@@ -59,7 +59,8 @@ func (d *Downloader) decrementUsage() {
 }
 
 func (d *Downloader) download() error {
-	if len(d.repo.getUrls()) == 0 {
+	urls := d.repo.getUrls()
+	if len(urls) == 0 {
 		return fmt.Errorf("repo %v has no urls", d.repoName)
 	}
 
@@ -70,7 +71,7 @@ func (d *Downloader) download() error {
 		proxyURL = nil
 	}
 
-	for _, u := range d.repo.getUrls() {
+	for _, u := range urls {
 		err := d.downloadFromUpstream(u, proxyURL)
 		if err != nil {
 			log.Printf("unable to download file %v: %v", d.key, err)

urls.go

@@ -44,11 +44,23 @@ func parseMirrorlistURLs(file *os.File) ([]string, error) {
 }
 
 func (r *Repo) getMirrorlistURLs() ([]string, error) {
-	if time.Since(r.LastMirrorlistCheck) < 5*time.Second {
+	const MirrorlistCheckInterval = 5*time.Second
+
+	if time.Since(r.LastMirrorlistCheck) < MirrorlistCheckInterval {
+		return r.URLs, nil
+	}
+
+	r.MirrorlistMutex.Lock()
+	defer r.MirrorlistMutex.Unlock()
+
+	// Test time again in case another routine already checked in the meantime
+	if time.Since(r.LastMirrorlistCheck) < MirrorlistCheckInterval {
 		return r.URLs, nil
 	}
 
-	r.LastMirrorlistCheck = time.Now()
+	defer func() {
+		r.LastMirrorlistCheck = time.Now()
+	}()
 
 	fileInfo, err := os.Stat(r.Mirrorlist)
 	if err != nil {