From 6bd2eb50468fa2e96c7be8895f1e72434ff6328d Mon Sep 17 00:00:00 2001
From: Tonis Tiigi <tonistiigi@gmail.com>
Date: Sat, 18 Sep 2021 21:04:16 -0700
Subject: [PATCH] resolver: use different mutext for handlers and hosts

hosts mutex is called on initialization, meaning `GetResolver` might
block if it is in the middle of auth exchange. This is currently bad
in the case where Job initialization needs to register a name before
timeout is reached.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
---
 util/resolver/authorizer.go | 19 ++++++++++---------
 util/resolver/pool.go       | 12 ++++++------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/util/resolver/authorizer.go b/util/resolver/authorizer.go
index 3b029b24..2766f215 100644
--- a/util/resolver/authorizer.go
+++ b/util/resolver/authorizer.go
@@ -25,11 +25,12 @@ import (
 type authHandlerNS struct {
 	counter int64 // needs to be 64bit aligned for 32bit systems
 
-	mu       sync.Mutex
-	handlers map[string]*authHandler
-	hosts    map[string][]docker.RegistryHost
-	sm       *session.Manager
-	g        flightcontrol.Group
+	handlers   map[string]*authHandler
+	muHandlers sync.Mutex
+	hosts      map[string][]docker.RegistryHost
+	muHosts    sync.Mutex
+	sm         *session.Manager
+	g          flightcontrol.Group
 }
 
 func newAuthHandlerNS(sm *session.Manager) *authHandlerNS {
@@ -118,8 +119,8 @@ func newDockerAuthorizer(client *http.Client, handlers *authHandlerNS, sm *sessi
 
 // Authorize handles auth request.
 func (a *dockerAuthorizer) Authorize(ctx context.Context, req *http.Request) error {
-	a.handlers.mu.Lock()
-	defer a.handlers.mu.Unlock()
+	a.handlers.muHandlers.Lock()
+	defer a.handlers.muHandlers.Unlock()
 
 	// skip if there is no auth handler
 	ah := a.handlers.get(ctx, req.URL.Host, a.sm, a.session)
@@ -141,8 +142,8 @@ func (a *dockerAuthorizer) getCredentials(host string) (sessionID, username, sec
 }
 
 func (a *dockerAuthorizer) AddResponses(ctx context.Context, responses []*http.Response) error {
-	a.handlers.mu.Lock()
-	defer a.handlers.mu.Unlock()
+	a.handlers.muHandlers.Lock()
+	defer a.handlers.muHandlers.Unlock()
 
 	last := responses[len(responses)-1]
 	host := last.Request.URL.Host
diff --git a/util/resolver/pool.go b/util/resolver/pool.go
index 5706ddb7..ad9725d0 100644
--- a/util/resolver/pool.go
+++ b/util/resolver/pool.go
@@ -40,7 +40,7 @@ func (p *Pool) gc() {
 	defer p.mu.Unlock()
 
 	for k, ns := range p.m {
-		ns.mu.Lock()
+		ns.muHandlers.Lock()
 		for key, h := range ns.handlers {
 			if time.Since(h.lastUsed) < 10*time.Minute {
 				continue
@@ -58,7 +58,7 @@ func (p *Pool) gc() {
 		if len(ns.handlers) == 0 {
 			delete(p.m, k)
 		}
-		ns.mu.Unlock()
+		ns.muHandlers.Unlock()
 	}
 
 	time.AfterFunc(5*time.Minute, p.gc)
@@ -128,9 +128,9 @@ func (r *Resolver) HostsFunc(host string) ([]docker.RegistryHost, error) {
 	return func(domain string) ([]docker.RegistryHost, error) {
 		v, err := r.handler.g.Do(context.TODO(), domain, func(ctx context.Context) (interface{}, error) {
 			// long lock not needed because flightcontrol.Do
-			r.handler.mu.Lock()
+			r.handler.muHosts.Lock()
 			v, ok := r.handler.hosts[domain]
-			r.handler.mu.Unlock()
+			r.handler.muHosts.Unlock()
 			if ok {
 				return v, nil
 			}
@@ -138,9 +138,9 @@ func (r *Resolver) HostsFunc(host string) ([]docker.RegistryHost, error) {
 			if err != nil {
 				return nil, err
 			}
-			r.handler.mu.Lock()
+			r.handler.muHosts.Lock()
 			r.handler.hosts[domain] = res
-			r.handler.mu.Unlock()
+			r.handler.muHosts.Unlock()
 			return res, nil
 		})
 		if err != nil || v == nil {