Skip to content

[Bug Report] Five data races in CubeMaster localcache, including a torn interface{} read that can corrupt memory #1375

Description

@dwin-gharibi

pkg/base/localcache mutates cache entries and counters without synchronization. The worst case is
put() overwriting a *util.CacheValue in place while Get() readers are dereferencing it, which
can tear the two-word interface{} and hand a caller a value whose type word and data word come from
different objects.

Environment

  • CubeSandbox version / commit: 5960c56 (master)
  • Host OS and kernel version: any
  • KVM info (modinfo kvm): n/a — control-plane only
  • Deployment mode: single-node / cluster
  • Relevant component: CubeMaster

Steps to Reproduce

cd CubeMaster && go test -race -count=1 ./...

9 data races and 9 failing tests, five of the races in production code.

Expected Behavior

Race-clean under the detector.

Actual Behavior

race 2: Write at localcache.go:201  | Previous write   (consecutiveFailNum)
race 4: Write at localcache.go:166  | Previous read at localcache.go:209   (CacheValue.Value)
race 5: Read  at localcache.go:122  | Previous write at localcache.go:167  (CacheValue.Expired/LastAccess)
race 7: Write at localcache.go:111  | Previous read at localcache.go:299   (chCacheExit)

Additional Context

Five distinct defects:

a. CacheValue mutated in place (worst). put() at :164-169 overwrites itm.Value,
itm.Expired and itm.LastAccess on an entry that Get() is concurrently reading at :122,
:139-140 and :148. The Lock()/Unlock() pairs at :145-147 and :161-163 protect only the
valueList LRU reordering, not the field access. util.CacheValue (util/util.go:14-19) has plain
fields.

Concurrency is by design: asyncRefresh (:219-233) spawns a goroutine into loadAndRefresh
put while request goroutines call Get. sharedCalls.Do dedupes concurrent loads for one key but
does not exclude concurrent readers.

Value is an interface{} — two machine words. A torn read pairs one value's type word with
another's data word, and the caller's type assertion then dereferences a mismatched pointer.

b. consecutiveFailNum mixes atomic and plain access. atomic.AddInt64 at :197 versus a plain
store at :201. errStrategy() reads it atomically at :305. The counter drives the cache's
degradation strategy.

c. curCacheSize read non-atomically at :183, written with atomic.AddInt64 at :165, :169,
:176.

d. Destroy() nils a channel a live goroutine selects onchCacheExit = nil at :111 versus
case <-localCache.chCacheExit: in errStrategy() at :299.

e. errStrategy() mutates the shared config structlocalCacheConfig.ExpiredUse = true at
:308 while Get() reads .ExpiredUse at :124.

The race detector is not wired into any test target, which is why these survived — see the separate
issue for adding -race to CI.

Metadata

Metadata

Labels

area/CubeMasterImpacts the CubeMatser (control plane)bugSomething isn't workinggoPull requests that update go codeneeds-triage

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions