Skip to content

Commit

Permalink
Fix potential race in capabilities test.
Browse files Browse the repository at this point in the history
  • Loading branch information
fancycode committed May 13, 2024
1 parent 5b305f6 commit ea0d31b
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 13 deletions.
19 changes: 11 additions & 8 deletions capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ const (
maxInvalidateInterval = time.Minute
)

// Can be overwritten by tests.
var getCapabilitiesNow = time.Now

type capabilitiesEntry struct {
nextUpdate time.Time
capabilities map[string]interface{}
Expand All @@ -59,6 +56,9 @@ type capabilitiesEntry struct {
type Capabilities struct {
mu sync.RWMutex

// Can be overwritten by tests.
getNow func() time.Time

version string
pool *HttpClientPool
entries map[string]*capabilitiesEntry
Expand All @@ -67,6 +67,8 @@ type Capabilities struct {

func NewCapabilities(version string, pool *HttpClientPool) (*Capabilities, error) {
result := &Capabilities{
getNow: time.Now,

version: version,
pool: pool,
entries: make(map[string]*capabilitiesEntry),
Expand Down Expand Up @@ -94,7 +96,7 @@ func (c *Capabilities) getCapabilities(key string) (map[string]interface{}, bool
c.mu.RLock()
defer c.mu.RUnlock()

now := getCapabilitiesNow()
now := c.getNow()
if entry, found := c.entries[key]; found && entry.nextUpdate.After(now) {
return entry.capabilities, true
}
Expand All @@ -103,22 +105,23 @@ func (c *Capabilities) getCapabilities(key string) (map[string]interface{}, bool
}

func (c *Capabilities) setCapabilities(key string, capabilities map[string]interface{}) {
now := getCapabilitiesNow()
c.mu.Lock()
defer c.mu.Unlock()

now := c.getNow()
entry := &capabilitiesEntry{
nextUpdate: now.Add(CapabilitiesCacheDuration),
capabilities: capabilities,
}

c.mu.Lock()
defer c.mu.Unlock()
c.entries[key] = entry
}

func (c *Capabilities) invalidateCapabilities(key string) {
c.mu.Lock()
defer c.mu.Unlock()

now := getCapabilitiesNow()
now := c.getNow()
if entry, found := c.nextInvalidate[key]; found && entry.After(now) {
return
}
Expand Down
17 changes: 12 additions & 5 deletions capabilities_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,20 @@ func NewCapabilitiesForTest(t *testing.T) (*url.URL, *Capabilities) {
return NewCapabilitiesForTestWithCallback(t, nil)
}

func SetCapabilitiesGetNow(t *testing.T, f func() time.Time) {
old := getCapabilitiesNow
func SetCapabilitiesGetNow(t *testing.T, capabilities *Capabilities, f func() time.Time) {
capabilities.mu.Lock()
defer capabilities.mu.Unlock()

old := capabilities.getNow

t.Cleanup(func() {
getCapabilitiesNow = old
capabilities.mu.Lock()
defer capabilities.mu.Unlock()

capabilities.getNow = old
})

getCapabilitiesNow = f
capabilities.getNow = f
}

func TestCapabilities(t *testing.T) {
Expand Down Expand Up @@ -248,7 +255,7 @@ func TestInvalidateCapabilities(t *testing.T) {
}

// At a later time, invalidating can be done again.
SetCapabilitiesGetNow(t, func() time.Time {
SetCapabilitiesGetNow(t, capabilities, func() time.Time {
return time.Now().Add(2 * time.Minute)
})

Expand Down

0 comments on commit ea0d31b

Please sign in to comment.