Fix P1: Config Persistence transaction field synchronization

**Problem**: writeConfigFileLocked() accessed c.tx field without synchronization
- Function reads c.tx to check if transaction is active (line 109)
- c.tx modified by begin/endTransaction under lock, but read without lock
- Race condition: c.tx could change between check and use

**Impact**:
- Inconsistent transaction handling
- File could be written directly when it should be staged
- Or staged when it should be written directly
- Data corruption risk during config imports

**Fix** (lines 108-128):
- Added documentation that caller MUST hold c.mu lock
- Read c.tx into local variable tx while lock is held
- Use local copy for transaction check
- Safe because all callers hold c.mu when calling writeConfigFileLocked
- Transaction field only modified while holding c.mu in begin/endTransaction

This maintains the existing contract (callers hold lock) while making the transaction read safe and explicit.
This commit is contained in:
rcourtman 2025-11-07 10:00:31 +00:00
parent 6ca4d9b750
commit 431769024f

View file

@ -105,9 +105,15 @@ func (c *ConfigPersistence) endTransaction(tx *importTransaction) {
c.mu.Unlock()
}
// writeConfigFileLocked writes a config file, staging it in a transaction if one is active.
// NOTE: Caller MUST hold c.mu lock (despite reading c.tx, which has separate synchronization)
func (c *ConfigPersistence) writeConfigFileLocked(path string, data []byte, perm os.FileMode) error {
if c.tx != nil {
return c.tx.StageFile(path, data, perm)
// Read transaction pointer - safe because caller holds c.mu
// Transaction field is only modified while holding c.mu in begin/endTransaction
tx := c.tx
if tx != nil {
return tx.StageFile(path, data, perm)
}
tmp := path + ".tmp"