manager/state/store.timedMutex bug

[Adirio]

The current implementation of timedMutex is modifying the atomic.Value after Unlocking, what means that if the context is swapped between m.Mutex.Unlock() and m.lockedAt.Store(time.Time{}), another timedMutex.Lock() call could store its time in m.lockedAt and it will be replaced by time.Time{} when the context switches back and finishes the timedMutex.Unlock() call.

I made the following script to prove it. I just swapped the storeLockDurationTimer.Update(lockedFor) line for a fmt.Println(...) that kind of guarantees there is gonna be a context swap and also allows to show the time.

package main

import (
	"sync/atomic"
	"fmt"
	"time"
	"sync"
)

type timedMutex struct {
	sync.Mutex
	lockedAt atomic.Value
}

func (m *timedMutex) Lock() {
	m.Mutex.Lock()
	m.lockedAt.Store(time.Now())
}

func (m *timedMutex) Unlock() {
	unlockedTimestamp := m.lockedAt.Load()
	m.Mutex.Unlock()
	lockedFor := time.Since(unlockedTimestamp.(time.Time))
	fmt.Println("Locked for", lockedFor)
	m.lockedAt.Store(time.Time{})
}

func main() {
	var mu timedMutex
	parallelLocks := 5

	for i := 1; i <= parallelLocks; i++ {
		go func(mu *timedMutex) {
			mu.Lock()
			time.Sleep(time.Second)
			mu.Unlock()
		}(&mu)
	}

	time.Sleep(time.Duration(2 + parallelLocks) * time.Second)
}

The expected result would be:

Locked for 1s
Locked for 1s
Locked for 1s
Locked for 1s
Locked for 1s

But what you actually get is:

Locked for 1s
Locked for 2562047h47m16.854775807s
Locked for 2562047h47m16.854775807s
Locked for 2562047h47m16.854775807s
Locked for 2562047h47m16.854775807s