Add separate cache lock to allow other goroutines to continue to read from cache while another updates the cert

Author: jellybeanfiend

proxy/proxy/client.go

@@ -81,9 +81,12 @@ type Client struct {
 
 	// The cfgCache holds the most recent connection configuration keyed by
 	// instance. Relevant functions are refreshCfg and cachedCfg. It is
-	// protected by cfgL.
+	// protected by cacheL.
 	cfgCache map[string]cacheEntry
-	cfgL     sync.RWMutex
+	cacheL   sync.RWMutex
+
+	// cfgL prevents multiple goroutines from contacting the Cloud SQL API at once.
+	cfgL sync.Mutex
 
 	// MaxConnections is the maximum number of connections to establish
 	// before refusing new connections. 0 means no limit.
@@ -161,24 +164,28 @@ func (c *Client) refreshCfg(instance string) (addr string, cfg *tls.Config, err
 		throttle = DefaultRefreshCfgThrottle
 	}
 
-	if old := c.cfgCache[instance]; time.Since(old.lastRefreshed) < throttle {
+	c.cacheL.Lock()
+	if c.cfgCache == nil {
+		c.cfgCache = make(map[string]cacheEntry)
+	}
+	old, oldok := c.cfgCache[instance]
+	c.cacheL.Unlock()
+
+	if oldok && time.Since(old.lastRefreshed) < throttle {
 		logging.Errorf("Throttling refreshCfg(%s): it was only called %v ago", instance, time.Since(old.lastRefreshed))
 		// Refresh was called too recently, just reuse the result.
 		return old.addr, old.cfg, old.err
 	}
 
-	if c.cfgCache == nil {
-		c.cfgCache = make(map[string]cacheEntry)
-	}
-
 	defer func() {
+		c.cacheL.Lock()
 		c.cfgCache[instance] = cacheEntry{
 			lastRefreshed: time.Now(),
-
-			err:  err,
-			addr: addr,
-			cfg:  cfg,
+			err:           err,
+			addr:          addr,
+			cfg:           cfg,
 		}
+		c.cacheL.Unlock()
 	}()
 
 	mycert, err := c.Certs.Local(instance)
@@ -198,13 +205,24 @@ func (c *Client) refreshCfg(instance string) (addr string, cfg *tls.Config, err
 		Certificates: []tls.Certificate{mycert},
 		RootCAs:      certs,
 	}
+
+	// Refresh cert 5 minutes before it expires.
+	timeToRefresh := cfg.Certificates[0].Leaf.NotAfter.Sub(time.Now()) - refreshCertBuffer
+	if timeToRefresh > 0 {
+		go func() {
+			<-time.After(timeToRefresh)
+			logging.Verbosef("Cert for instance %s will expire soon, refreshing now.", instance)
+			c.refreshCfg(instance)
+		}()
+	}
+
 	return fmt.Sprintf("%s:%d", addr, c.Port), cfg, nil
 }
 
 func (c *Client) cachedCfg(instance string) (string, *tls.Config) {
-	c.cfgL.RLock()
+	c.cacheL.RLock()
 	ret, ok := c.cfgCache[instance]
-	c.cfgL.RUnlock()
+	c.cacheL.RUnlock()
 
 	// Don't waste time returning an expired/invalid cert.
 	if !ok || ret.err != nil || time.Now().After(ret.cfg.Certificates[0].Leaf.NotAfter) {
@@ -229,14 +247,6 @@ func (c *Client) Dial(instance string) (net.Conn, error) {
 		return nil, err
 	}
 
-	// Refresh cert 5 minutes before it expires.
-	timer := time.NewTimer(cfg.Certificates[0].Leaf.NotAfter.Sub(time.Now()) - refreshCertBuffer)
-	go func() {
-		<-timer.C
-		logging.Verbosef("Cert for instance %s will expire soon, refreshing now.", instance)
-		c.refreshCfg(instance)
-	}()
-
 	return c.tryConnect(addr, cfg)
 }