From aef9daebf8cf77c16a0a544c75da2c073bda954d Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 10 Mar 2026 12:42:02 -0700 Subject: [PATCH] capture v.nm locally in CompactByIndex to close TOCTOU race A bare nil check on v.nm followed by v.nm.Sync() has a race window where CommitCompact can set v.nm = nil between the two. Snapshot the pointer into a local variable so the nil check and Sync operate on the same reference. --- weed/storage/volume_vacuum.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/weed/storage/volume_vacuum.go b/weed/storage/volume_vacuum.go index aed10d9f5..e65539cf6 100644 --- a/weed/storage/volume_vacuum.go +++ b/weed/storage/volume_vacuum.go @@ -111,13 +111,18 @@ func (v *Volume) CompactByIndex(opts *CompactOptions) error { if v.DataBackend == nil { return fmt.Errorf("volume %d backend is empty remote:%v", v.Id, v.HasRemoteFile()) } - if v.nm == nil { + // Capture v.nm locally: CommitCompact can set v.nm = nil without holding + // dataFileAccessLock, so a bare nil check followed by v.nm.Sync() has a + // TOCTOU race. The atomic isCompacting/isCommitCompacting flags are + // advisory and do not fully prevent overlap. + nm := v.nm + if nm == nil { return fmt.Errorf("volume %d needle map is nil", v.Id) } if err := v.DataBackend.Sync(); err != nil { glog.V(0).Infof("compact2 failed to sync volume dat %d: %v", v.Id, err) } - if err := v.nm.Sync(); err != nil { + if err := nm.Sync(); err != nil { glog.V(0).Infof("compact2 failed to sync volume idx %d: %v", v.Id, err) }