fix(filer): avoid ReaderCache WaitGroup reuse race between reads and destroy (#10190)

filer: count reader on cacher waitgroup under the map lock

The read's wg.Add(1) ran after ReadChunkAt released the ReaderCache lock, so a
concurrent destroy() (error eviction, LRU, or UnCache) could start wg.Wait() on
a zero counter and then race the Add - a WaitGroup reuse that trips -race and
can panic. Move the Add under the lock, before the cacher can leave the
downloaders map, so a destroy is always ordered against a counted read.
This commit is contained in:
Chris Lu
2026-07-01 21:17:46 -07:00
committed by GitHub
parent 155140bed8
commit ece4f42ecd
+6 -1
View File
@@ -116,6 +116,10 @@ func (rc *ReaderCache) ReadChunkAt(ctx context.Context, buffer []byte, fileId st
rc.Lock()
continue
}
// Count this read on the cacher before releasing the map lock, so a
// concurrent destroy() (error eviction here, LRU, or UnCache) cannot
// start wg.Wait() on a zero counter while this read is about to register.
cacher.wg.Add(1)
rc.Unlock()
n, err := cacher.readChunkAt(ctx, buffer, offset)
if n > 0 || err != nil {
@@ -157,6 +161,7 @@ func (rc *ReaderCache) ReadChunkAt(ctx context.Context, buffer []byte, fileId st
go cacher.startCaching()
<-cacher.cacheStartedCh
rc.downloaders[fileId] = cacher
cacher.wg.Add(1)
rc.Unlock()
return cacher.readChunkAt(ctx, buffer, offset)
@@ -302,8 +307,8 @@ func (s *SingleChunkCacher) destroy() {
// It waits for the download to complete if it's still in progress.
// The ctx parameter allows the reader to cancel its wait (but the download continues
// for other readers - see comment in startCaching about shared resource semantics).
// The caller must s.wg.Add(1) under the ReaderCache lock before calling; this only releases it.
func (s *SingleChunkCacher) readChunkAt(ctx context.Context, buf []byte, offset int64) (int, error) {
s.wg.Add(1)
defer s.wg.Done()
// Wait for download to complete, but allow reader cancellation.