Skip to content

Commit 0fdd179

Browse files
committed
fix(libkb): take the write lock in the remaining cache setters
SetImplicitTeamConflictInfoCacher, SetImplicitTeamCacher and SetKVRevisionCache mutated GlobalContext fields while holding only cacheMu.RLock(), which races with concurrent getters. Also document why the annotated team cache ages entries from the load start rather than the load finish.
1 parent cd6db7d commit 0fdd179

2 files changed

Lines changed: 9 additions & 9 deletions

File tree

go/libkb/globals.go

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -754,8 +754,8 @@ func (g *GlobalContext) GetImplicitTeamConflictInfoCacher() LRUer {
754754
}
755755

756756
func (g *GlobalContext) SetImplicitTeamConflictInfoCacher(l LRUer) {
757-
g.cacheMu.RLock()
758-
defer g.cacheMu.RUnlock()
757+
g.cacheMu.Lock()
758+
defer g.cacheMu.Unlock()
759759
g.itciCacher = l
760760
}
761761

@@ -766,8 +766,8 @@ func (g *GlobalContext) GetImplicitTeamCacher() MemLRUer {
766766
}
767767

768768
func (g *GlobalContext) SetImplicitTeamCacher(l MemLRUer) {
769-
g.cacheMu.RLock()
770-
defer g.cacheMu.RUnlock()
769+
g.cacheMu.Lock()
770+
defer g.cacheMu.Unlock()
771771
g.iteamCacher = l
772772
}
773773

@@ -778,9 +778,6 @@ func (g *GlobalContext) GetAnnotatedTeamCacher() AnnotatedTeamCacher {
778778
}
779779

780780
func (g *GlobalContext) SetAnnotatedTeamCacher(c AnnotatedTeamCacher) {
781-
// a write needs the write lock: GetAnnotatedTeamCacher runs from
782-
// NotifyRouter.HandleTeamChangedByID on every team notification. The
783-
// neighbouring setters take RLock here too, and are equally wrong.
784781
g.cacheMu.Lock()
785782
defer g.cacheMu.Unlock()
786783
g.annotatedTeamCacher = c
@@ -793,8 +790,8 @@ func (g *GlobalContext) GetKVRevisionCache() KVRevisionCacher {
793790
}
794791

795792
func (g *GlobalContext) SetKVRevisionCache(kvr KVRevisionCacher) {
796-
g.cacheMu.RLock()
797-
defer g.cacheMu.RUnlock()
793+
g.cacheMu.Lock()
794+
defer g.cacheMu.Unlock()
798795
g.kvRevisionCache = kvr
799796
}
800797

go/teams/annotated_cache.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,9 @@ func (c *annotatedTeamCache) load(mctx libkb.MetaContext, teamID keybase1.TeamID
151151
}()
152152
}
153153

154+
// Age the entry from when the load started, not when it finished: the
155+
// data describes the server state as of the request, so a slow loader
156+
// must not buy the result extra TTL it has already spent being stale.
154157
startedAt := mctx.G().Clock().Now()
155158
res, err = loader(mctx.Ctx(), mctx.G(), teamID)
156159

0 commit comments

Comments
 (0)