From 925e844a6a97a173d9c11904c4d561043985d1ec Mon Sep 17 00:00:00 2001 From: Jason Song Date: Thu, 22 Aug 2024 12:30:36 +0800 Subject: [PATCH 01/19] feat: globallock --- go.mod | 3 + go.sum | 7 +++ modules/globallock/globallock.go | 13 +++++ modules/globallock/memory_locker.go | 50 +++++++++++++++++ modules/globallock/redis_locker.go | 86 +++++++++++++++++++++++++++++ 5 files changed, 159 insertions(+) create mode 100644 modules/globallock/globallock.go create mode 100644 modules/globallock/memory_locker.go create mode 100644 modules/globallock/redis_locker.go diff --git a/go.mod b/go.mod index f5c189893f4d5..1d05a1cec6659 100644 --- a/go.mod +++ b/go.mod @@ -201,6 +201,7 @@ require ( github.com/go-openapi/strfmt v0.23.0 // indirect github.com/go-openapi/swag v0.23.0 // indirect github.com/go-openapi/validate v0.24.0 // indirect + github.com/go-redsync/redsync/v4 v4.13.0 // indirect github.com/go-webauthn/x v0.1.9 // indirect github.com/goccy/go-json v0.10.3 // indirect github.com/golang-jwt/jwt/v4 v4.5.0 // indirect @@ -218,7 +219,9 @@ require ( github.com/gorilla/mux v1.8.1 // indirect github.com/gorilla/securecookie v1.1.2 // indirect github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 // indirect + github.com/hashicorp/errwrap v1.1.0 // indirect github.com/hashicorp/go-cleanhttp v0.5.2 // indirect + github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/hashicorp/go-retryablehttp v0.7.7 // indirect github.com/hashicorp/hcl v1.0.0 // indirect github.com/imdario/mergo v0.3.16 // indirect diff --git a/go.sum b/go.sum index f1780fada7981..0882d6be225a7 100644 --- a/go.sum +++ b/go.sum @@ -342,6 +342,8 @@ github.com/go-openapi/swag v0.23.0/go.mod h1:esZ8ITTYEsH1V2trKHjAN8Ai7xHb8RV+YSZ github.com/go-openapi/validate v0.24.0 h1:LdfDKwNbpB6Vn40xhTdNZAnfLECL81w+VX3BumrGD58= github.com/go-openapi/validate v0.24.0/go.mod h1:iyeX1sEufmv3nPbBdX3ieNviWnOZaJ1+zquzJEf2BAQ= github.com/go-redis/redis v6.15.2+incompatible/go.mod h1:NAIEuMOZ/fxfXJIrKDQDz8wamY7mA7PouImQ2Jvg6kA= +github.com/go-redsync/redsync/v4 v4.13.0 h1:49X6GJfnbLGaIpBBREM/zA4uIMDXKAh1NDkvQ1EkZKA= +github.com/go-redsync/redsync/v4 v4.13.0/go.mod h1:HMW4Q224GZQz6x1Xc7040Yfgacukdzu7ifTDAKiyErQ= github.com/go-sql-driver/mysql v1.4.1/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= github.com/go-sql-driver/mysql v1.8.1 h1:LedoTUt/eveggdHS9qUFC1EFSa8bU2+1pZjSRpvNJ1Y= github.com/go-sql-driver/mysql v1.8.1/go.mod h1:wEBSXgmK//2ZFJyE+qWnIsVGmvmEKlqwuVSjsCm7DZg= @@ -449,10 +451,15 @@ github.com/h2non/gock v1.2.0 h1:K6ol8rfrRkUOefooBC8elXoaNGYkpp7y2qcxGG6BzUE= github.com/h2non/gock v1.2.0/go.mod h1:tNhoxHYW2W42cYkYb1WqzdbYIieALC99kpYr7rH/BQk= github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 h1:2VTzZjLZBgl62/EtslCrtky5vbi9dd7HrQPQIx6wqiw= github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542/go.mod h1:Ow0tF8D4Kplbc8s8sSb3V2oUCygFHVp8gC3Dn6U4MNI= +github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= +github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I= +github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48= github.com/hashicorp/go-hclog v1.6.3 h1:Qr2kF+eVWjTiYmU7Y31tYlP1h0q/X3Nl3tPGdaB11/k= github.com/hashicorp/go-hclog v1.6.3/go.mod h1:W4Qnvbt70Wk/zYJryRzDRU/4r0kIg0PVHBcfoyhpF5M= +github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= +github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= github.com/hashicorp/go-retryablehttp v0.7.7 h1:C8hUCYzor8PIfXHa4UrZkU4VvK8o9ISHxT2Q8+VepXU= github.com/hashicorp/go-retryablehttp v0.7.7/go.mod h1:pkQpWZeYWskR+D1tR2O5OcBFOxfA7DoAO6xtkuQnHTk= github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= diff --git a/modules/globallock/globallock.go b/modules/globallock/globallock.go new file mode 100644 index 0000000000000..b44265310d227 --- /dev/null +++ b/modules/globallock/globallock.go @@ -0,0 +1,13 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package globallock + +import ( + "context" +) + +type Locker interface { + Lock(ctx context.Context, key string) (context.Context, func(), error) + TryLock(ctx context.Context, key string) (bool, context.Context, func(), error) +} diff --git a/modules/globallock/memory_locker.go b/modules/globallock/memory_locker.go new file mode 100644 index 0000000000000..b2250ef05e38d --- /dev/null +++ b/modules/globallock/memory_locker.go @@ -0,0 +1,50 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package globallock + +import ( + "context" + "sync" + "time" +) + +type memoryLocker struct { + locks sync.Map +} + +func (l *memoryLocker) Lock(ctx context.Context, key string) (context.Context, func(), error) { + if l.tryLock(key) { + return ctx, func() { + l.locks.Delete(key) + }, nil + } + ticker := time.NewTicker(time.Millisecond * 100) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return ctx, func() {}, ctx.Err() + case <-ticker.C: + if l.tryLock(key) { + return ctx, func() { + l.locks.Delete(key) + }, nil + } + } + } +} + +func (l *memoryLocker) TryLock(ctx context.Context, key string) (bool, context.Context, func(), error) { + if l.tryLock(key) { + return true, ctx, func() { + l.locks.Delete(key) + }, nil + } + return false, ctx, func() {}, nil +} + +func (l *memoryLocker) tryLock(key string) bool { + _, loaded := l.locks.LoadOrStore(key, struct{}{}) + return !loaded +} diff --git a/modules/globallock/redis_locker.go b/modules/globallock/redis_locker.go new file mode 100644 index 0000000000000..fd40aca2546d3 --- /dev/null +++ b/modules/globallock/redis_locker.go @@ -0,0 +1,86 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package globallock + +import ( + "context" + "fmt" + "sync" + "time" + + "github.com/go-redsync/redsync/v4" +) + +type redisLocker struct { + rs *redsync.Redsync + + mutexM sync.Map +} + +func (l *redisLocker) Lock(ctx context.Context, key string) (context.Context, func(), error) { + return l.lock(ctx, key, 0) +} + +func (l *redisLocker) TryLock(ctx context.Context, key string) (bool, context.Context, func(), error) { + ctx, f, err := l.lock(ctx, key, 1) + return err == nil, ctx, f, err +} + +type redisMutex struct { + mutex *redsync.Mutex + cancel context.CancelCauseFunc +} + +func (l *redisLocker) lock(ctx context.Context, key string, tries int) (context.Context, func(), error) { + var options []redsync.Option + if tries > 0 { + options = append(options, redsync.WithTries(tries)) + } + mutex := l.rs.NewMutex(key, options...) + if err := mutex.LockContext(ctx); err != nil { + return ctx, func() {}, err + } + + ctx, cancel := context.WithCancelCause(ctx) + + l.mutexM.Store(key, &redisMutex{ + mutex: mutex, + cancel: cancel, + }) + + return ctx, func() { + l.mutexM.Delete(key) + + // It's safe to ignore the error here, + // if the lock is not released, it will be released automatically after the lock expires. + // Do not call mutex.UnlockContext(ctx) here, or it will fail to unlock when ctx has timed out. + _, _ = mutex.Unlock() + cancel(fmt.Errorf("lock released")) + }, nil +} + +func (l *redisLocker) extend() { + toExtend := make([]*redisMutex, 0) + l.mutexM.Range(func(_, value interface{}) bool { + m := value.(*redisMutex) + + // Extend the lock if it is not expired. + // Although the mutex will be removed from the map before it is unlocked, + // it still can be expired because of a failed extension. + // If it happens, the cancel function should have been called, + // so it does not need to be extended anymore. + if time.Now().Before(m.mutex.Until()) { + toExtend = append(toExtend, m) + } + return true + }) + for _, v := range toExtend { + if ok, err := v.mutex.Extend(); !ok { + v.cancel(err) + } + } + + // TODO: a better duration + time.AfterFunc(5*time.Second, l.extend) +} From 15c41b4bb2783a1e054eeb4fc5687acfde287dcd Mon Sep 17 00:00:00 2001 From: Jason Song Date: Thu, 22 Aug 2024 15:18:04 +0800 Subject: [PATCH 02/19] test: TestLocker --- modules/globallock/globallock_test.go | 114 ++++++++++++++++++++++++++ modules/globallock/memory_locker.go | 15 ++++ modules/globallock/redis_locker.go | 46 +++++++++-- 3 files changed, 167 insertions(+), 8 deletions(-) create mode 100644 modules/globallock/globallock_test.go diff --git a/modules/globallock/globallock_test.go b/modules/globallock/globallock_test.go new file mode 100644 index 0000000000000..62a46ee02e169 --- /dev/null +++ b/modules/globallock/globallock_test.go @@ -0,0 +1,114 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package globallock + +import ( + "context" + "os" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLocker(t *testing.T) { + t.Run("redis", func(t *testing.T) { + if os.Getenv("CI") == "" { + t.Skip("Skip test for local development") + return + } + testLocker(t, NewRedisLocker("redis://127.0.0.1:6379/0")) + }) + t.Run("memory", func(t *testing.T) { + testLocker(t, NewMemoryLocker()) + }) +} + +func testLocker(t *testing.T, locker Locker) { + t.Run("lock", func(t *testing.T) { + parentCtx := context.Background() + ctx, unlock, err := locker.Lock(parentCtx, "test") + defer unlock() + + assert.NotEqual(t, parentCtx, ctx) // new context should be returned + assert.NoError(t, err) + + func() { + parentCtx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + ctx, unlock, err := locker.Lock(parentCtx, "test") + defer unlock() + + assert.Error(t, err) + assert.Equal(t, parentCtx, ctx) // should return the same context + }() + + unlock() + assert.Error(t, ctx.Err()) + unlock() // should be safe to call multiple times + + func() { + _, unlock, err := locker.Lock(context.Background(), "test") + defer unlock() + + assert.NoError(t, err) + }() + }) + + t.Run("try lock", func(t *testing.T) { + parentCtx := context.Background() + ok, ctx, unlock, err := locker.TryLock(parentCtx, "test") + defer unlock() + + assert.True(t, ok) + assert.NotEqual(t, parentCtx, ctx) // new context should be returned + assert.NoError(t, err) + + func() { + parentCtx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + ok, ctx, unlock, err := locker.TryLock(parentCtx, "test") + defer unlock() + + assert.False(t, ok) + assert.NoError(t, err) + assert.Equal(t, parentCtx, ctx) // should return the same context + }() + + unlock() + assert.Error(t, ctx.Err()) + unlock() // should be safe to call multiple times + + func() { + ok, _, unlock, _ := locker.TryLock(context.Background(), "test") + defer unlock() + + assert.True(t, ok) + }() + }) + + t.Run("wait and acquired", func(t *testing.T) { + ctx := context.Background() + ctx, unlock, err := locker.Lock(ctx, "test") + require.NoError(t, err) + + wg := &sync.WaitGroup{} + wg.Add(1) + go func() { + defer wg.Done() + started := time.Now() + _, unlock, err := locker.Lock(context.Background(), "test") // should be blocked for seconds + defer unlock() + assert.Greater(t, time.Since(started), time.Second) + assert.NoError(t, err) + }() + + time.Sleep(2 * time.Second) + unlock() + + wg.Wait() + }) +} diff --git a/modules/globallock/memory_locker.go b/modules/globallock/memory_locker.go index b2250ef05e38d..e24cf459a425b 100644 --- a/modules/globallock/memory_locker.go +++ b/modules/globallock/memory_locker.go @@ -5,6 +5,7 @@ package globallock import ( "context" + "fmt" "sync" "time" ) @@ -13,12 +14,21 @@ type memoryLocker struct { locks sync.Map } +var _ Locker = &memoryLocker{} + +func NewMemoryLocker() Locker { + return &memoryLocker{} +} + func (l *memoryLocker) Lock(ctx context.Context, key string) (context.Context, func(), error) { if l.tryLock(key) { + ctx, cancel := context.WithCancelCause(ctx) return ctx, func() { l.locks.Delete(key) + cancel(fmt.Errorf("unlock")) }, nil } + ticker := time.NewTicker(time.Millisecond * 100) defer ticker.Stop() for { @@ -27,8 +37,10 @@ func (l *memoryLocker) Lock(ctx context.Context, key string) (context.Context, f return ctx, func() {}, ctx.Err() case <-ticker.C: if l.tryLock(key) { + ctx, cancel := context.WithCancelCause(ctx) return ctx, func() { l.locks.Delete(key) + cancel(fmt.Errorf("unlock")) }, nil } } @@ -37,10 +49,13 @@ func (l *memoryLocker) Lock(ctx context.Context, key string) (context.Context, f func (l *memoryLocker) TryLock(ctx context.Context, key string) (bool, context.Context, func(), error) { if l.tryLock(key) { + ctx, cancel := context.WithCancelCause(ctx) return true, ctx, func() { + cancel(fmt.Errorf("unlock")) l.locks.Delete(key) }, nil } + return false, ctx, func() {}, nil } diff --git a/modules/globallock/redis_locker.go b/modules/globallock/redis_locker.go index fd40aca2546d3..fd5f0ae31ab8f 100644 --- a/modules/globallock/redis_locker.go +++ b/modules/globallock/redis_locker.go @@ -5,11 +5,20 @@ package globallock import ( "context" + "errors" "fmt" "sync" "time" + "code.gitea.io/gitea/modules/nosql" + "github.com/go-redsync/redsync/v4" + "github.com/go-redsync/redsync/v4/redis/goredis/v9" +) + +const ( + redisLockKeyPrefix = "gitea:globallock:" + redisLockExpiry = 30 * time.Second ) type redisLocker struct { @@ -18,12 +27,30 @@ type redisLocker struct { mutexM sync.Map } +var _ Locker = &redisLocker{} + +func NewRedisLocker(connection string) Locker { + l := &redisLocker{ + rs: redsync.New( + goredis.NewPool( + nosql.GetManager().GetRedisClient(connection), + ), + ), + } + l.startExtend() + + return l +} + func (l *redisLocker) Lock(ctx context.Context, key string) (context.Context, func(), error) { return l.lock(ctx, key, 0) } func (l *redisLocker) TryLock(ctx context.Context, key string) (bool, context.Context, func(), error) { ctx, f, err := l.lock(ctx, key, 1) + if errors.Is(err, redsync.ErrFailed) { + return false, ctx, f, nil + } return err == nil, ctx, f, err } @@ -33,11 +60,13 @@ type redisMutex struct { } func (l *redisLocker) lock(ctx context.Context, key string, tries int) (context.Context, func(), error) { - var options []redsync.Option + options := []redsync.Option{ + redsync.WithExpiry(redisLockExpiry), + } if tries > 0 { options = append(options, redsync.WithTries(tries)) } - mutex := l.rs.NewMutex(key, options...) + mutex := l.rs.NewMutex(redisLockKeyPrefix+key, options...) if err := mutex.LockContext(ctx); err != nil { return ctx, func() {}, err } @@ -56,11 +85,11 @@ func (l *redisLocker) lock(ctx context.Context, key string, tries int) (context. // if the lock is not released, it will be released automatically after the lock expires. // Do not call mutex.UnlockContext(ctx) here, or it will fail to unlock when ctx has timed out. _, _ = mutex.Unlock() - cancel(fmt.Errorf("lock released")) + cancel(fmt.Errorf("unlock")) }, nil } -func (l *redisLocker) extend() { +func (l *redisLocker) startExtend() { toExtend := make([]*redisMutex, 0) l.mutexM.Range(func(_, value interface{}) bool { m := value.(*redisMutex) @@ -70,9 +99,11 @@ func (l *redisLocker) extend() { // it still can be expired because of a failed extension. // If it happens, the cancel function should have been called, // so it does not need to be extended anymore. - if time.Now().Before(m.mutex.Until()) { - toExtend = append(toExtend, m) + if time.Now().After(m.mutex.Until()) { + return true } + + toExtend = append(toExtend, m) return true }) for _, v := range toExtend { @@ -81,6 +112,5 @@ func (l *redisLocker) extend() { } } - // TODO: a better duration - time.AfterFunc(5*time.Second, l.extend) + time.AfterFunc(redisLockExpiry/2, l.startExtend) } From 2895437744fd4274518235b9ac3380c023c6caf3 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Thu, 22 Aug 2024 17:49:38 +0800 Subject: [PATCH 03/19] test: add tests --- modules/globallock/globallock_test.go | 52 ++++++++++++++++++++++++--- modules/globallock/redis_locker.go | 16 ++++++--- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/modules/globallock/globallock_test.go b/modules/globallock/globallock_test.go index 62a46ee02e169..31f51132175be 100644 --- a/modules/globallock/globallock_test.go +++ b/modules/globallock/globallock_test.go @@ -10,20 +10,36 @@ import ( "testing" "time" + "github.com/go-redsync/redsync/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestLocker(t *testing.T) { t.Run("redis", func(t *testing.T) { + url := "redis://127.0.0.1:6379/0" if os.Getenv("CI") == "" { - t.Skip("Skip test for local development") - return + // Make it possible to run tests against a local redis instance + url = os.Getenv("TEST_REDIS_URL") + if url == "" { + t.Skip("TEST_REDIS_URL not set and not running in CI") + return + } } - testLocker(t, NewRedisLocker("redis://127.0.0.1:6379/0")) + oldExpiry := redisLockExpiry + redisLockExpiry = 5 * time.Second // make it shorter for testing + defer func() { + redisLockExpiry = oldExpiry + }() + + locker := NewRedisLocker(url) + testLocker(t, locker) + testRedisLocker(t, locker.(*redisLocker)) }) t.Run("memory", func(t *testing.T) { - testLocker(t, NewMemoryLocker()) + locker := NewMemoryLocker() + testLocker(t, locker) + testMemoryLocker(t, locker.(*memoryLocker)) }) } @@ -112,3 +128,31 @@ func testLocker(t *testing.T, locker Locker) { wg.Wait() }) } + +// testMemoryLocker does specific tests for memoryLocker +func testMemoryLocker(t *testing.T, locker *memoryLocker) { + // nothing to do +} + +// testRedisLocker does specific tests for redisLocker +func testRedisLocker(t *testing.T, locker *redisLocker) { + t.Run("missing extension", func(t *testing.T) { + ctx, unlock, err := locker.Lock(context.Background(), "test") + defer unlock() + require.NoError(t, err) + + // It simulates that there are some problems with extending like network issues or redis server down. + v, ok := locker.mutexM.Load("test") + require.True(t, ok) + m := v.(*redisMutex) + _, _ = m.mutex.Unlock() // unlock it to make it impossible to extend + + select { + case <-time.After(redisLockExpiry + time.Second): + t.Errorf("lock should be expired") + case <-ctx.Done(): + var errTaken *redsync.ErrTaken + assert.ErrorAs(t, context.Cause(ctx), &errTaken) + } + }) +} diff --git a/modules/globallock/redis_locker.go b/modules/globallock/redis_locker.go index fd5f0ae31ab8f..9a76bb3bb1f0e 100644 --- a/modules/globallock/redis_locker.go +++ b/modules/globallock/redis_locker.go @@ -16,10 +16,11 @@ import ( "github.com/go-redsync/redsync/v4/redis/goredis/v9" ) -const ( - redisLockKeyPrefix = "gitea:globallock:" - redisLockExpiry = 30 * time.Second -) +const redisLockKeyPrefix = "gitea:globallock:" + +// redisLockExpiry is the default expiry time for a lock. +// Define it as a variable to make it possible to change it in tests. +var redisLockExpiry = 30 * time.Second type redisLocker struct { rs *redsync.Redsync @@ -48,7 +49,12 @@ func (l *redisLocker) Lock(ctx context.Context, key string) (context.Context, fu func (l *redisLocker) TryLock(ctx context.Context, key string) (bool, context.Context, func(), error) { ctx, f, err := l.lock(ctx, key, 1) - if errors.Is(err, redsync.ErrFailed) { + + var ( + errTaken *redsync.ErrTaken + errNodeTaken *redsync.ErrNodeTaken + ) + if errors.As(err, &errTaken) || errors.As(err, &errNodeTaken) { return false, ctx, f, nil } return err == nil, ctx, f, err From 0b0424fe39932222c28f30a5a27ba587a958f776 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Aug 2024 12:11:52 +0800 Subject: [PATCH 04/19] feat: ReleaseFunc --- modules/globallock/globallock.go | 6 ++- modules/globallock/globallock_test.go | 65 +++++++++++++++++---------- modules/globallock/memory_locker.go | 27 ++++++----- modules/globallock/redis_locker.go | 22 +++++---- 4 files changed, 76 insertions(+), 44 deletions(-) diff --git a/modules/globallock/globallock.go b/modules/globallock/globallock.go index b44265310d227..cf8f413d60913 100644 --- a/modules/globallock/globallock.go +++ b/modules/globallock/globallock.go @@ -8,6 +8,8 @@ import ( ) type Locker interface { - Lock(ctx context.Context, key string) (context.Context, func(), error) - TryLock(ctx context.Context, key string) (bool, context.Context, func(), error) + Lock(ctx context.Context, key string) (context.Context, ReleaseFunc, error) + TryLock(ctx context.Context, key string) (bool, context.Context, ReleaseFunc, error) } + +type ReleaseFunc func() context.Context diff --git a/modules/globallock/globallock_test.go b/modules/globallock/globallock_test.go index 31f51132175be..d2dcd76413018 100644 --- a/modules/globallock/globallock_test.go +++ b/modules/globallock/globallock_test.go @@ -46,8 +46,8 @@ func TestLocker(t *testing.T) { func testLocker(t *testing.T, locker Locker) { t.Run("lock", func(t *testing.T) { parentCtx := context.Background() - ctx, unlock, err := locker.Lock(parentCtx, "test") - defer unlock() + ctx, release, err := locker.Lock(parentCtx, "test") + defer release() assert.NotEqual(t, parentCtx, ctx) // new context should be returned assert.NoError(t, err) @@ -55,20 +55,20 @@ func testLocker(t *testing.T, locker Locker) { func() { parentCtx, cancel := context.WithTimeout(context.Background(), time.Second) defer cancel() - ctx, unlock, err := locker.Lock(parentCtx, "test") - defer unlock() + ctx, release, err := locker.Lock(parentCtx, "test") + defer release() assert.Error(t, err) assert.Equal(t, parentCtx, ctx) // should return the same context }() - unlock() + release() assert.Error(t, ctx.Err()) - unlock() // should be safe to call multiple times + release() // should be safe to call multiple times func() { - _, unlock, err := locker.Lock(context.Background(), "test") - defer unlock() + _, release, err := locker.Lock(context.Background(), "test") + defer release() assert.NoError(t, err) }() @@ -76,8 +76,8 @@ func testLocker(t *testing.T, locker Locker) { t.Run("try lock", func(t *testing.T) { parentCtx := context.Background() - ok, ctx, unlock, err := locker.TryLock(parentCtx, "test") - defer unlock() + ok, ctx, release, err := locker.TryLock(parentCtx, "test") + defer release() assert.True(t, ok) assert.NotEqual(t, parentCtx, ctx) // new context should be returned @@ -86,21 +86,21 @@ func testLocker(t *testing.T, locker Locker) { func() { parentCtx, cancel := context.WithTimeout(context.Background(), time.Second) defer cancel() - ok, ctx, unlock, err := locker.TryLock(parentCtx, "test") - defer unlock() + ok, ctx, release, err := locker.TryLock(parentCtx, "test") + defer release() assert.False(t, ok) assert.NoError(t, err) assert.Equal(t, parentCtx, ctx) // should return the same context }() - unlock() + release() assert.Error(t, ctx.Err()) - unlock() // should be safe to call multiple times + release() // should be safe to call multiple times func() { - ok, _, unlock, _ := locker.TryLock(context.Background(), "test") - defer unlock() + ok, _, release, _ := locker.TryLock(context.Background(), "test") + defer release() assert.True(t, ok) }() @@ -108,7 +108,7 @@ func testLocker(t *testing.T, locker Locker) { t.Run("wait and acquired", func(t *testing.T) { ctx := context.Background() - ctx, unlock, err := locker.Lock(ctx, "test") + ctx, release, err := locker.Lock(ctx, "test") require.NoError(t, err) wg := &sync.WaitGroup{} @@ -116,17 +116,36 @@ func testLocker(t *testing.T, locker Locker) { go func() { defer wg.Done() started := time.Now() - _, unlock, err := locker.Lock(context.Background(), "test") // should be blocked for seconds - defer unlock() + _, release, err := locker.Lock(context.Background(), "test") // should be blocked for seconds + defer release() assert.Greater(t, time.Since(started), time.Second) assert.NoError(t, err) }() time.Sleep(2 * time.Second) - unlock() + release() wg.Wait() }) + + t.Run("continue after release", func(t *testing.T) { + ctx := context.Background() + + ctxBeforeLock := ctx + ctx, release, err := locker.Lock(ctx, "test") + + require.NoError(t, err) + assert.NoError(t, ctx.Err()) + assert.NotEqual(t, ctxBeforeLock, ctx) + + ctxBeforeRelease := ctx + ctx = release() + + assert.NoError(t, ctx.Err()) + assert.Error(t, ctxBeforeRelease.Err()) + + // so it can continue with ctx to do more work + }) } // testMemoryLocker does specific tests for memoryLocker @@ -137,15 +156,15 @@ func testMemoryLocker(t *testing.T, locker *memoryLocker) { // testRedisLocker does specific tests for redisLocker func testRedisLocker(t *testing.T, locker *redisLocker) { t.Run("missing extension", func(t *testing.T) { - ctx, unlock, err := locker.Lock(context.Background(), "test") - defer unlock() + ctx, release, err := locker.Lock(context.Background(), "test") + defer release() require.NoError(t, err) // It simulates that there are some problems with extending like network issues or redis server down. v, ok := locker.mutexM.Load("test") require.True(t, ok) m := v.(*redisMutex) - _, _ = m.mutex.Unlock() // unlock it to make it impossible to extend + _, _ = m.mutex.Unlock() // release it to make it impossible to extend select { case <-time.After(redisLockExpiry + time.Second): diff --git a/modules/globallock/memory_locker.go b/modules/globallock/memory_locker.go index e24cf459a425b..de2ddbb797b70 100644 --- a/modules/globallock/memory_locker.go +++ b/modules/globallock/memory_locker.go @@ -20,12 +20,15 @@ func NewMemoryLocker() Locker { return &memoryLocker{} } -func (l *memoryLocker) Lock(ctx context.Context, key string) (context.Context, func(), error) { +func (l *memoryLocker) Lock(ctx context.Context, key string) (context.Context, ReleaseFunc, error) { + originalCtx := ctx + if l.tryLock(key) { ctx, cancel := context.WithCancelCause(ctx) - return ctx, func() { + return ctx, func() context.Context { l.locks.Delete(key) - cancel(fmt.Errorf("unlock")) + cancel(fmt.Errorf("release")) + return originalCtx }, nil } @@ -34,29 +37,33 @@ func (l *memoryLocker) Lock(ctx context.Context, key string) (context.Context, f for { select { case <-ctx.Done(): - return ctx, func() {}, ctx.Err() + return ctx, func() context.Context { return originalCtx }, ctx.Err() case <-ticker.C: if l.tryLock(key) { ctx, cancel := context.WithCancelCause(ctx) - return ctx, func() { + return ctx, func() context.Context { l.locks.Delete(key) - cancel(fmt.Errorf("unlock")) + cancel(fmt.Errorf("release")) + return originalCtx }, nil } } } } -func (l *memoryLocker) TryLock(ctx context.Context, key string) (bool, context.Context, func(), error) { +func (l *memoryLocker) TryLock(ctx context.Context, key string) (bool, context.Context, ReleaseFunc, error) { + originalCtx := ctx + if l.tryLock(key) { ctx, cancel := context.WithCancelCause(ctx) - return true, ctx, func() { - cancel(fmt.Errorf("unlock")) + return true, ctx, func() context.Context { + cancel(fmt.Errorf("release")) l.locks.Delete(key) + return originalCtx }, nil } - return false, ctx, func() {}, nil + return false, ctx, func() context.Context { return originalCtx }, nil } func (l *memoryLocker) tryLock(key string) bool { diff --git a/modules/globallock/redis_locker.go b/modules/globallock/redis_locker.go index 9a76bb3bb1f0e..1d31fb63df236 100644 --- a/modules/globallock/redis_locker.go +++ b/modules/globallock/redis_locker.go @@ -43,11 +43,11 @@ func NewRedisLocker(connection string) Locker { return l } -func (l *redisLocker) Lock(ctx context.Context, key string) (context.Context, func(), error) { +func (l *redisLocker) Lock(ctx context.Context, key string) (context.Context, ReleaseFunc, error) { return l.lock(ctx, key, 0) } -func (l *redisLocker) TryLock(ctx context.Context, key string) (bool, context.Context, func(), error) { +func (l *redisLocker) TryLock(ctx context.Context, key string) (bool, context.Context, ReleaseFunc, error) { ctx, f, err := l.lock(ctx, key, 1) var ( @@ -65,7 +65,9 @@ type redisMutex struct { cancel context.CancelCauseFunc } -func (l *redisLocker) lock(ctx context.Context, key string, tries int) (context.Context, func(), error) { +func (l *redisLocker) lock(ctx context.Context, key string, tries int) (context.Context, ReleaseFunc, error) { + originalCtx := ctx + options := []redsync.Option{ redsync.WithExpiry(redisLockExpiry), } @@ -74,7 +76,7 @@ func (l *redisLocker) lock(ctx context.Context, key string, tries int) (context. } mutex := l.rs.NewMutex(redisLockKeyPrefix+key, options...) if err := mutex.LockContext(ctx); err != nil { - return ctx, func() {}, err + return ctx, func() context.Context { return originalCtx }, err } ctx, cancel := context.WithCancelCause(ctx) @@ -84,14 +86,16 @@ func (l *redisLocker) lock(ctx context.Context, key string, tries int) (context. cancel: cancel, }) - return ctx, func() { + return ctx, func() context.Context { l.mutexM.Delete(key) // It's safe to ignore the error here, - // if the lock is not released, it will be released automatically after the lock expires. - // Do not call mutex.UnlockContext(ctx) here, or it will fail to unlock when ctx has timed out. + // if it failed to unlock, it will be released automatically after the lock expires. + // Do not call mutex.UnlockContext(ctx) here, or it will fail to release when ctx has timed out. _, _ = mutex.Unlock() - cancel(fmt.Errorf("unlock")) + + cancel(fmt.Errorf("release")) + return originalCtx }, nil } @@ -101,7 +105,7 @@ func (l *redisLocker) startExtend() { m := value.(*redisMutex) // Extend the lock if it is not expired. - // Although the mutex will be removed from the map before it is unlocked, + // Although the mutex will be removed from the map before it is releaseed, // it still can be expired because of a failed extension. // If it happens, the cancel function should have been called, // so it does not need to be extended anymore. From 97a4c23864036b4ad02bf8397df3c6b960e18104 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Aug 2024 12:26:12 +0800 Subject: [PATCH 05/19] fix: releaseOnce --- modules/globallock/globallock_test.go | 27 +++++++++++++++++++++++++-- modules/globallock/memory_locker.go | 21 +++++++++++++++------ modules/globallock/redis_locker.go | 17 ++++++++++------- 3 files changed, 50 insertions(+), 15 deletions(-) diff --git a/modules/globallock/globallock_test.go b/modules/globallock/globallock_test.go index d2dcd76413018..0affddd6a63da 100644 --- a/modules/globallock/globallock_test.go +++ b/modules/globallock/globallock_test.go @@ -64,7 +64,6 @@ func testLocker(t *testing.T, locker Locker) { release() assert.Error(t, ctx.Err()) - release() // should be safe to call multiple times func() { _, release, err := locker.Lock(context.Background(), "test") @@ -96,7 +95,6 @@ func testLocker(t *testing.T, locker Locker) { release() assert.Error(t, ctx.Err()) - release() // should be safe to call multiple times func() { ok, _, release, _ := locker.TryLock(context.Background(), "test") @@ -146,6 +144,31 @@ func testLocker(t *testing.T, locker Locker) { // so it can continue with ctx to do more work }) + + t.Run("multiple release", func(t *testing.T) { + ctx := context.Background() + + _, release1, err := locker.Lock(ctx, "test") + require.NoError(t, err) + + release1() + + _, release2, err := locker.Lock(ctx, "test") + defer release2() + require.NoError(t, err) + + // Call release1 again, + // it should not panic or block, + // and it shouldn't affect the other lock + release1() + + ok, _, release3, err := locker.TryLock(ctx, "test") + defer release3() + require.NoError(t, err) + // It should be able to acquire the lock; + // otherwise, it means the lock has been released by release1 + assert.False(t, ok) + }) } // testMemoryLocker does specific tests for memoryLocker diff --git a/modules/globallock/memory_locker.go b/modules/globallock/memory_locker.go index de2ddbb797b70..5186a214e3ffd 100644 --- a/modules/globallock/memory_locker.go +++ b/modules/globallock/memory_locker.go @@ -25,9 +25,12 @@ func (l *memoryLocker) Lock(ctx context.Context, key string) (context.Context, R if l.tryLock(key) { ctx, cancel := context.WithCancelCause(ctx) + releaseOnce := sync.Once{} return ctx, func() context.Context { - l.locks.Delete(key) - cancel(fmt.Errorf("release")) + releaseOnce.Do(func() { + l.locks.Delete(key) + cancel(fmt.Errorf("release")) + }) return originalCtx }, nil } @@ -41,9 +44,12 @@ func (l *memoryLocker) Lock(ctx context.Context, key string) (context.Context, R case <-ticker.C: if l.tryLock(key) { ctx, cancel := context.WithCancelCause(ctx) + releaseOnce := sync.Once{} return ctx, func() context.Context { - l.locks.Delete(key) - cancel(fmt.Errorf("release")) + releaseOnce.Do(func() { + l.locks.Delete(key) + cancel(fmt.Errorf("release")) + }) return originalCtx }, nil } @@ -56,9 +62,12 @@ func (l *memoryLocker) TryLock(ctx context.Context, key string) (bool, context.C if l.tryLock(key) { ctx, cancel := context.WithCancelCause(ctx) + releaseOnce := sync.Once{} return true, ctx, func() context.Context { - cancel(fmt.Errorf("release")) - l.locks.Delete(key) + releaseOnce.Do(func() { + cancel(fmt.Errorf("release")) + l.locks.Delete(key) + }) return originalCtx }, nil } diff --git a/modules/globallock/redis_locker.go b/modules/globallock/redis_locker.go index 1d31fb63df236..52f40a5bdf61f 100644 --- a/modules/globallock/redis_locker.go +++ b/modules/globallock/redis_locker.go @@ -86,15 +86,18 @@ func (l *redisLocker) lock(ctx context.Context, key string, tries int) (context. cancel: cancel, }) + releaseOnce := sync.Once{} return ctx, func() context.Context { - l.mutexM.Delete(key) + releaseOnce.Do(func() { + l.mutexM.Delete(key) - // It's safe to ignore the error here, - // if it failed to unlock, it will be released automatically after the lock expires. - // Do not call mutex.UnlockContext(ctx) here, or it will fail to release when ctx has timed out. - _, _ = mutex.Unlock() + // It's safe to ignore the error here, + // if it failed to unlock, it will be released automatically after the lock expires. + // Do not call mutex.UnlockContext(ctx) here, or it will fail to release when ctx has timed out. + _, _ = mutex.Unlock() - cancel(fmt.Errorf("release")) + cancel(fmt.Errorf("release")) + }) return originalCtx }, nil } @@ -105,7 +108,7 @@ func (l *redisLocker) startExtend() { m := value.(*redisMutex) // Extend the lock if it is not expired. - // Although the mutex will be removed from the map before it is releaseed, + // Although the mutex will be removed from the map before it is released, // it still can be expired because of a failed extension. // If it happens, the cancel function should have been called, // so it does not need to be extended anymore. From a1eea5c8d9e38f44c18e9429708ad8bce1c0d046 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Aug 2024 12:31:57 +0800 Subject: [PATCH 06/19] feat: ErrLockReleased --- modules/globallock/globallock.go | 4 ++++ modules/globallock/memory_locker.go | 7 +++---- modules/globallock/redis_locker.go | 3 +-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/modules/globallock/globallock.go b/modules/globallock/globallock.go index cf8f413d60913..70ea4b7126d1c 100644 --- a/modules/globallock/globallock.go +++ b/modules/globallock/globallock.go @@ -5,6 +5,7 @@ package globallock import ( "context" + "fmt" ) type Locker interface { @@ -13,3 +14,6 @@ type Locker interface { } type ReleaseFunc func() context.Context + +// ErrLockReleased is used as context cause when a lock is released +var ErrLockReleased = fmt.Errorf("lock released") diff --git a/modules/globallock/memory_locker.go b/modules/globallock/memory_locker.go index 5186a214e3ffd..fb1fc79bd0bdb 100644 --- a/modules/globallock/memory_locker.go +++ b/modules/globallock/memory_locker.go @@ -5,7 +5,6 @@ package globallock import ( "context" - "fmt" "sync" "time" ) @@ -29,7 +28,7 @@ func (l *memoryLocker) Lock(ctx context.Context, key string) (context.Context, R return ctx, func() context.Context { releaseOnce.Do(func() { l.locks.Delete(key) - cancel(fmt.Errorf("release")) + cancel(ErrLockReleased) }) return originalCtx }, nil @@ -48,7 +47,7 @@ func (l *memoryLocker) Lock(ctx context.Context, key string) (context.Context, R return ctx, func() context.Context { releaseOnce.Do(func() { l.locks.Delete(key) - cancel(fmt.Errorf("release")) + cancel(ErrLockReleased) }) return originalCtx }, nil @@ -65,7 +64,7 @@ func (l *memoryLocker) TryLock(ctx context.Context, key string) (bool, context.C releaseOnce := sync.Once{} return true, ctx, func() context.Context { releaseOnce.Do(func() { - cancel(fmt.Errorf("release")) + cancel(ErrLockReleased) l.locks.Delete(key) }) return originalCtx diff --git a/modules/globallock/redis_locker.go b/modules/globallock/redis_locker.go index 52f40a5bdf61f..d4058f0bb283d 100644 --- a/modules/globallock/redis_locker.go +++ b/modules/globallock/redis_locker.go @@ -6,7 +6,6 @@ package globallock import ( "context" "errors" - "fmt" "sync" "time" @@ -96,7 +95,7 @@ func (l *redisLocker) lock(ctx context.Context, key string, tries int) (context. // Do not call mutex.UnlockContext(ctx) here, or it will fail to release when ctx has timed out. _, _ = mutex.Unlock() - cancel(fmt.Errorf("release")) + cancel(ErrLockReleased) }) return originalCtx }, nil From 85c5137a0600243bfcba3bd0844f4c8830c14b30 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Aug 2024 14:18:29 +0800 Subject: [PATCH 07/19] chore: rename locker --- modules/globallock/globallock.go | 19 ------ modules/globallock/locker.go | 59 +++++++++++++++++++ .../{globallock_test.go => locker_test.go} | 0 3 files changed, 59 insertions(+), 19 deletions(-) delete mode 100644 modules/globallock/globallock.go create mode 100644 modules/globallock/locker.go rename modules/globallock/{globallock_test.go => locker_test.go} (100%) diff --git a/modules/globallock/globallock.go b/modules/globallock/globallock.go deleted file mode 100644 index 70ea4b7126d1c..0000000000000 --- a/modules/globallock/globallock.go +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright 2024 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package globallock - -import ( - "context" - "fmt" -) - -type Locker interface { - Lock(ctx context.Context, key string) (context.Context, ReleaseFunc, error) - TryLock(ctx context.Context, key string) (bool, context.Context, ReleaseFunc, error) -} - -type ReleaseFunc func() context.Context - -// ErrLockReleased is used as context cause when a lock is released -var ErrLockReleased = fmt.Errorf("lock released") diff --git a/modules/globallock/locker.go b/modules/globallock/locker.go new file mode 100644 index 0000000000000..1ce3330c7e3e8 --- /dev/null +++ b/modules/globallock/locker.go @@ -0,0 +1,59 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package globallock + +import ( + "context" + "fmt" +) + +type Locker interface { + // Lock tries to acquire a lock for the given key, it blocks until the lock is acquired or the context is canceled. + // + // Lock returns a new context which should be used in the following code. + // The new context will be canceled when the lock is released or lost - yes, it's possible to lose a lock. + // For example, it lost the connection to the redis server while holding the lock. + // If it fails to acquire the lock, the returned context will be the same as the input context. + // + // Lock returns a ReleaseFunc to release the lock, it cannot be nil. + // It's always safe to call this function even if it fails to acquire the lock, and it will do nothing in that case. + // And it's also safe to call it multiple times, but it will only release the lock once. + // That's why it's called ReleaseFunc, not UnlockFunc. + // But be aware that it's not safe to not call it at all; it could lead to a memory leak. + // So a recommended pattern is to use defer to call it: + // ctx, release, err := locker.Lock(ctx, "key") + // if err != nil { + // return err + // } + // defer release() + // The ReleaseFunc will return the original context which was used to acquire the lock. + // It's useful when you want to continue to do something after releasing the lock. + // At that time, the ctx will be canceled, and you can use the returned context by the ReleaseFunc to continue: + // ctx, release, err := locker.Lock(ctx, "key") + // if err != nil { + // return err + // } + // doSomething(ctx) + // ctx = release() + // doSomethingElse(ctx) + // Please ignore it and use `defer release()` instead if you don't need this, to avoid forgetting to release the lock. + // + // Lock returns an error if failed to acquire the lock. + // Be aware that even the context is not canceled, it's still possible to fail to acquire the lock. + // For example, redis is down, or it reached the maximum number of tries. + Lock(ctx context.Context, key string) (context.Context, ReleaseFunc, error) + + // TryLock tries to acquire a lock for the given key, it returns immediately. + // It follows the same pattern as Lock, but it doesn't block. + // And if it fails to acquire the lock because it's already locked, not other reasons like redis is down, + // it will return false without any error. + TryLock(ctx context.Context, key string) (bool, context.Context, ReleaseFunc, error) +} + +// ReleaseFunc is a function that releases a lock. +// It returns the original context which was used to acquire the lock. +type ReleaseFunc func() context.Context + +// ErrLockReleased is used as context cause when a lock is released +var ErrLockReleased = fmt.Errorf("lock released") diff --git a/modules/globallock/globallock_test.go b/modules/globallock/locker_test.go similarity index 100% rename from modules/globallock/globallock_test.go rename to modules/globallock/locker_test.go From bd2a860c6cd9c05e6d9428f9de9a37a8f11b3f57 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Aug 2024 15:19:25 +0800 Subject: [PATCH 08/19] feat: support default locker --- modules/globallock/globallock.go | 66 ++++++++++++++++++++ modules/globallock/globallock_test.go | 87 +++++++++++++++++++++++++++ 2 files changed, 153 insertions(+) create mode 100644 modules/globallock/globallock.go create mode 100644 modules/globallock/globallock_test.go diff --git a/modules/globallock/globallock.go b/modules/globallock/globallock.go new file mode 100644 index 0000000000000..ca23592d63f24 --- /dev/null +++ b/modules/globallock/globallock.go @@ -0,0 +1,66 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package globallock + +import ( + "context" + "sync" +) + +var ( + defaultLocker Locker + initOnce sync.Once + initFunc = func() { + // TODO: read the setting and initialize the default locker. + // Before implementing this, don't use it. + } // define initFunc as a variable to make it possible to change it in tests +) + +// DefaultLocker returns the default locker. +func DefaultLocker() Locker { + initOnce.Do(func() { + initFunc() + }) + return defaultLocker +} + +// Lock tries to acquire a lock for the given key, it uses the default locker. +// Read the documentation of Locker.Lock for more information about the behavior. +func Lock(ctx context.Context, key string) (context.Context, ReleaseFunc, error) { + return DefaultLocker().Lock(ctx, key) +} + +// TryLock tries to acquire a lock for the given key, it uses the default locker. +// Read the documentation of Locker.TryLock for more information about the behavior. +func TryLock(ctx context.Context, key string) (bool, context.Context, ReleaseFunc, error) { + return DefaultLocker().TryLock(ctx, key) +} + +// LockAndDo tries to acquire a lock for the given key and then calls the given function. +// It uses the default locker. +func LockAndDo(ctx context.Context, key string, f func(context.Context) error) error { + ctx, release, err := Lock(ctx, key) + if err != nil { + return err + } + defer release() + + return f(ctx) +} + +// TryLockAndDo tries to acquire a lock for the given key and then calls the given function. +// It uses the default locker, and it will return false if failed to acquire the lock. +func TryLockAndDo(ctx context.Context, key string, f func(context.Context) error) (bool, error) { + ok, ctx, release, err := TryLock(ctx, key) + if err != nil { + return false, err + } + defer release() + + if !ok { + return false, nil + } + + return true, f(ctx) +} diff --git a/modules/globallock/globallock_test.go b/modules/globallock/globallock_test.go new file mode 100644 index 0000000000000..6ec18c7f0bcaf --- /dev/null +++ b/modules/globallock/globallock_test.go @@ -0,0 +1,87 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package globallock + +import ( + "context" + "os" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLockAndDo(t *testing.T) { + t.Run("redis", func(t *testing.T) { + url := "redis://127.0.0.1:6379/0" + if os.Getenv("CI") == "" { + // Make it possible to run tests against a local redis instance + url = os.Getenv("TEST_REDIS_URL") + if url == "" { + t.Skip("TEST_REDIS_URL not set and not running in CI") + return + } + } + + initOnce = sync.Once{} + initFunc = func() { + defaultLocker = NewRedisLocker(url) + } + + testLockAndDo(t) + }) + t.Run("memory", func(t *testing.T) { + initOnce = sync.Once{} + initFunc = func() { + defaultLocker = NewMemoryLocker() + } + + testLockAndDo(t) + }) +} + +func testLockAndDo(t *testing.T) { + const ( + duration = 2 * time.Second + concurrency = 100 + ) + + ctx := context.Background() + count := 0 + wg := sync.WaitGroup{} + wg.Add(concurrency) + for i := 0; i < concurrency; i++ { + go func() { + defer wg.Done() + err := LockAndDo(ctx, "test", func(ctx context.Context) error { + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(duration / concurrency): + count++ + } + return nil + }) + require.NoError(t, err) + }() + } + + ok, err := TryLockAndDo(ctx, "test", func(ctx context.Context) error { + return nil + }) + assert.False(t, ok) + assert.NoError(t, err) + + wg.Wait() + + ok, err = TryLockAndDo(ctx, "test", func(ctx context.Context) error { + return nil + }) + assert.True(t, ok) + assert.NoError(t, err) + + assert.Equal(t, concurrency, count) +} From f4d545cdf689ef25c242908dcaf2fdcd7a0d6e44 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Aug 2024 15:22:14 +0800 Subject: [PATCH 09/19] test: improve TestLockAndDo --- modules/globallock/globallock_test.go | 34 +++++++++------------------ 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/modules/globallock/globallock_test.go b/modules/globallock/globallock_test.go index 6ec18c7f0bcaf..9c1d8c09bc0ff 100644 --- a/modules/globallock/globallock_test.go +++ b/modules/globallock/globallock_test.go @@ -8,7 +8,6 @@ import ( "os" "sync" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -44,10 +43,7 @@ func TestLockAndDo(t *testing.T) { } func testLockAndDo(t *testing.T) { - const ( - duration = 2 * time.Second - concurrency = 100 - ) + const concurrency = 1000 ctx := context.Background() count := 0 @@ -57,31 +53,23 @@ func testLockAndDo(t *testing.T) { go func() { defer wg.Done() err := LockAndDo(ctx, "test", func(ctx context.Context) error { - select { - case <-ctx.Done(): - return ctx.Err() - case <-time.After(duration / concurrency): - count++ - } + count++ + + // It's impossible to acquire the lock inner the function + ok, err := TryLockAndDo(ctx, "test", func(ctx context.Context) error { + assert.Fail(t, "should not acquire the lock") + return nil + }) + assert.False(t, ok) + assert.NoError(t, err) + return nil }) require.NoError(t, err) }() } - ok, err := TryLockAndDo(ctx, "test", func(ctx context.Context) error { - return nil - }) - assert.False(t, ok) - assert.NoError(t, err) - wg.Wait() - ok, err = TryLockAndDo(ctx, "test", func(ctx context.Context) error { - return nil - }) - assert.True(t, ok) - assert.NoError(t, err) - assert.Equal(t, concurrency, count) } From 0289b78f5c24db744a7ac64e94ecb2640b4ae64e Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Aug 2024 15:24:18 +0800 Subject: [PATCH 10/19] chore: improve comment --- modules/globallock/globallock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/globallock/globallock.go b/modules/globallock/globallock.go index ca23592d63f24..707d169f05c6e 100644 --- a/modules/globallock/globallock.go +++ b/modules/globallock/globallock.go @@ -38,7 +38,7 @@ func TryLock(ctx context.Context, key string) (bool, context.Context, ReleaseFun } // LockAndDo tries to acquire a lock for the given key and then calls the given function. -// It uses the default locker. +// It uses the default locker, and it will return an error if failed to acquire the lock. func LockAndDo(ctx context.Context, key string, f func(context.Context) error) error { ctx, release, err := Lock(ctx, key) if err != nil { From 570860f5c6d0c27c252370827911196d9ef99e1c Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Aug 2024 16:23:08 +0800 Subject: [PATCH 11/19] docs: always defer release() --- modules/globallock/locker.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/globallock/locker.go b/modules/globallock/locker.go index 1ce3330c7e3e8..b0764cd71cdb1 100644 --- a/modules/globallock/locker.go +++ b/modules/globallock/locker.go @@ -34,6 +34,7 @@ type Locker interface { // if err != nil { // return err // } + // defer release() // doSomething(ctx) // ctx = release() // doSomethingElse(ctx) From cae33ec27c82b43f261c68956c4422d8ca44b6c9 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Aug 2024 16:34:47 +0800 Subject: [PATCH 12/19] chore: lint --- modules/globallock/locker_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/globallock/locker_test.go b/modules/globallock/locker_test.go index 0affddd6a63da..f4a3118207f2c 100644 --- a/modules/globallock/locker_test.go +++ b/modules/globallock/locker_test.go @@ -106,7 +106,7 @@ func testLocker(t *testing.T, locker Locker) { t.Run("wait and acquired", func(t *testing.T) { ctx := context.Background() - ctx, release, err := locker.Lock(ctx, "test") + _, release, err := locker.Lock(ctx, "test") require.NoError(t, err) wg := &sync.WaitGroup{} From 302b099585f4ad76fa92a1cfebd1bc0ceef0ecc8 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Aug 2024 16:45:41 +0800 Subject: [PATCH 13/19] chore: go mod tidy --- go.mod | 2 +- go.sum | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 1d05a1cec6659..69695fa178733 100644 --- a/go.mod +++ b/go.mod @@ -49,6 +49,7 @@ require ( github.com/go-git/go-billy/v5 v5.5.0 github.com/go-git/go-git/v5 v5.12.0 github.com/go-ldap/ldap/v3 v3.4.6 + github.com/go-redsync/redsync/v4 v4.13.0 github.com/go-sql-driver/mysql v1.8.1 github.com/go-swagger/go-swagger v0.31.0 github.com/go-testfixtures/testfixtures/v3 v3.11.0 @@ -201,7 +202,6 @@ require ( github.com/go-openapi/strfmt v0.23.0 // indirect github.com/go-openapi/swag v0.23.0 // indirect github.com/go-openapi/validate v0.24.0 // indirect - github.com/go-redsync/redsync/v4 v4.13.0 // indirect github.com/go-webauthn/x v0.1.9 // indirect github.com/goccy/go-json v0.10.3 // indirect github.com/golang-jwt/jwt/v4 v4.5.0 // indirect diff --git a/go.sum b/go.sum index 0882d6be225a7..510ef8479de18 100644 --- a/go.sum +++ b/go.sum @@ -342,6 +342,12 @@ github.com/go-openapi/swag v0.23.0/go.mod h1:esZ8ITTYEsH1V2trKHjAN8Ai7xHb8RV+YSZ github.com/go-openapi/validate v0.24.0 h1:LdfDKwNbpB6Vn40xhTdNZAnfLECL81w+VX3BumrGD58= github.com/go-openapi/validate v0.24.0/go.mod h1:iyeX1sEufmv3nPbBdX3ieNviWnOZaJ1+zquzJEf2BAQ= github.com/go-redis/redis v6.15.2+incompatible/go.mod h1:NAIEuMOZ/fxfXJIrKDQDz8wamY7mA7PouImQ2Jvg6kA= +github.com/go-redis/redis v6.15.9+incompatible h1:K0pv1D7EQUjfyoMql+r/jZqCLizCGKFlFgcHWWmHQjg= +github.com/go-redis/redis v6.15.9+incompatible/go.mod h1:NAIEuMOZ/fxfXJIrKDQDz8wamY7mA7PouImQ2Jvg6kA= +github.com/go-redis/redis/v7 v7.4.1 h1:PASvf36gyUpr2zdOUS/9Zqc80GbM+9BDyiJSJDDOrTI= +github.com/go-redis/redis/v7 v7.4.1/go.mod h1:JDNMw23GTyLNC4GZu9njt15ctBQVn7xjRfnwdHj/Dcg= +github.com/go-redis/redis/v8 v8.11.5 h1:AcZZR7igkdvfVmQTPnu9WE37LRrO/YrBH5zWyjDC0oI= +github.com/go-redis/redis/v8 v8.11.5/go.mod h1:gREzHqY1hg6oD9ngVRbLStwAWKhA0FEgq8Jd4h5lpwo= github.com/go-redsync/redsync/v4 v4.13.0 h1:49X6GJfnbLGaIpBBREM/zA4uIMDXKAh1NDkvQ1EkZKA= github.com/go-redsync/redsync/v4 v4.13.0/go.mod h1:HMW4Q224GZQz6x1Xc7040Yfgacukdzu7ifTDAKiyErQ= github.com/go-sql-driver/mysql v1.4.1/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= @@ -399,6 +405,8 @@ github.com/golang/snappy v0.0.1/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEW github.com/golang/snappy v0.0.2/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/golang/snappy v0.0.4 h1:yAGX7huGHXlcLOEtBnF4w7FQwA26wojNCwOYAEhLjQM= github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= +github.com/gomodule/redigo v1.8.9 h1:Sl3u+2BI/kk+VEatbj0scLdrFhjPmbxOc1myhDP41ws= +github.com/gomodule/redigo v1.8.9/go.mod h1:7ArFNvsTjH8GMMzB4uy1snslv2BwmginuMs06a1uzZE= github.com/google/btree v1.1.2 h1:xf4v41cLI2Z6FxbKm+8Bu+m8ifhj15JuZ9sa0jZCMUU= github.com/google/btree v1.1.2/go.mod h1:qOPhT0dTNdNzV6Z/lhRX0YXUafgPLFUh+gZMl761Gm4= github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= @@ -681,6 +689,8 @@ github.com/quasoft/websspi v1.1.2/go.mod h1:HmVdl939dQ0WIXZhyik+ARdI03M6bQzaSEKc github.com/rcrowley/go-metrics v0.0.0-20190826022208-cac0b30c2563/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4= github.com/redis/go-redis/v9 v9.6.0 h1:NLck+Rab3AOTHw21CGRpvQpgTrAU4sgdCswqGtlhGRA= github.com/redis/go-redis/v9 v9.6.0/go.mod h1:hdY0cQFCN4fnSYT6TkisLufl/4W5UIXyv0b/CLO2V2M= +github.com/redis/rueidis v1.0.19 h1:s65oWtotzlIFN8eMPhyYwxlwLR1lUdhza2KtWprKYSo= +github.com/redis/rueidis v1.0.19/go.mod h1:8B+r5wdnjwK3lTFml5VtxjzGOQAC+5UmujoD12pDrEo= github.com/remyoudompheng/bigfft v0.0.0-20200410134404-eec4a21b6bb0 h1:OdAsTTz6OkFY5QxjkYwrChwuRruF69c169dPK26NUlk= github.com/remyoudompheng/bigfft v0.0.0-20200410134404-eec4a21b6bb0/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo= github.com/rhysd/actionlint v1.7.1 h1:WJaDzyT1StBWVKGSsZPYnbV0HF9Y9/vD6KFdZQL42qE= @@ -772,6 +782,8 @@ github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/stvp/tempredis v0.0.0-20181119212430-b82af8480203 h1:QVqDTf3h2WHt08YuiTGPZLls0Wq99X9bWd0Q5ZSBesM= +github.com/stvp/tempredis v0.0.0-20181119212430-b82af8480203/go.mod h1:oqN97ltKNihBbwlX8dLpwxCl3+HnXKV/R0e+sRLd9C8= github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= github.com/syndtr/goleveldb v1.0.0 h1:fBdIW9lB4Iz0n9khmH8w27SJ3QEJ7+IgjPEwGSZiFdE= From a07b7765a171f3c589ec740f409a90fe5ffda467 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Aug 2024 16:58:04 +0800 Subject: [PATCH 14/19] chore: lint --- modules/globallock/redis_locker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/globallock/redis_locker.go b/modules/globallock/redis_locker.go index d4058f0bb283d..508ac4f2266d9 100644 --- a/modules/globallock/redis_locker.go +++ b/modules/globallock/redis_locker.go @@ -103,7 +103,7 @@ func (l *redisLocker) lock(ctx context.Context, key string, tries int) (context. func (l *redisLocker) startExtend() { toExtend := make([]*redisMutex, 0) - l.mutexM.Range(func(_, value interface{}) bool { + l.mutexM.Range(func(_, value any) bool { m := value.(*redisMutex) // Extend the lock if it is not expired. From 4613c40ac35db11fe08d2647459df24bd266f4f2 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Aug 2024 17:41:23 +0800 Subject: [PATCH 15/19] fix: close --- modules/globallock/globallock_test.go | 11 +++++++++++ modules/globallock/locker_test.go | 13 ++++++++++++- modules/globallock/redis_locker.go | 20 ++++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/modules/globallock/globallock_test.go b/modules/globallock/globallock_test.go index 9c1d8c09bc0ff..fd4c3323c1c74 100644 --- a/modules/globallock/globallock_test.go +++ b/modules/globallock/globallock_test.go @@ -25,14 +25,25 @@ func TestLockAndDo(t *testing.T) { } } + oldDefaultLocker := defaultLocker + defer func() { + defaultLocker = oldDefaultLocker + }() + initOnce = sync.Once{} initFunc = func() { defaultLocker = NewRedisLocker(url) } testLockAndDo(t) + require.NoError(t, defaultLocker.(*redisLocker).Close()) }) t.Run("memory", func(t *testing.T) { + oldDefaultLocker := defaultLocker + defer func() { + defaultLocker = oldDefaultLocker + }() + initOnce = sync.Once{} initFunc = func() { defaultLocker = NewMemoryLocker() diff --git a/modules/globallock/locker_test.go b/modules/globallock/locker_test.go index f4a3118207f2c..15a3c65bb070f 100644 --- a/modules/globallock/locker_test.go +++ b/modules/globallock/locker_test.go @@ -35,6 +35,7 @@ func TestLocker(t *testing.T) { locker := NewRedisLocker(url) testLocker(t, locker) testRedisLocker(t, locker.(*redisLocker)) + require.NoError(t, locker.(*redisLocker).Close()) }) t.Run("memory", func(t *testing.T) { locker := NewMemoryLocker() @@ -178,7 +179,17 @@ func testMemoryLocker(t *testing.T, locker *memoryLocker) { // testRedisLocker does specific tests for redisLocker func testRedisLocker(t *testing.T, locker *redisLocker) { - t.Run("missing extension", func(t *testing.T) { + defer func() { + // This case should be tested at the end. + // Otherwise, it will affect other tests. + t.Run("close", func(t *testing.T) { + assert.NoError(t, locker.Close()) + _, _, err := locker.Lock(context.Background(), "test") + assert.Error(t, err) + }) + }() + + t.Run("failed extend", func(t *testing.T) { ctx, release, err := locker.Lock(context.Background(), "test") defer release() require.NoError(t, err) diff --git a/modules/globallock/redis_locker.go b/modules/globallock/redis_locker.go index 508ac4f2266d9..89c13fd97c7fe 100644 --- a/modules/globallock/redis_locker.go +++ b/modules/globallock/redis_locker.go @@ -6,7 +6,9 @@ package globallock import ( "context" "errors" + "fmt" "sync" + "sync/atomic" "time" "code.gitea.io/gitea/modules/nosql" @@ -25,6 +27,7 @@ type redisLocker struct { rs *redsync.Redsync mutexM sync.Map + closed atomic.Bool } var _ Locker = &redisLocker{} @@ -59,12 +62,25 @@ func (l *redisLocker) TryLock(ctx context.Context, key string) (bool, context.Co return err == nil, ctx, f, err } +// Close closes the locker. +// It will stop extending the locks and refuse to acquire new locks. +// In actual use, it is not necessary to call this function. +// But it's useful in tests to release resources. +func (l *redisLocker) Close() error { + l.closed.Store(true) + return nil +} + type redisMutex struct { mutex *redsync.Mutex cancel context.CancelCauseFunc } func (l *redisLocker) lock(ctx context.Context, key string, tries int) (context.Context, ReleaseFunc, error) { + if l.closed.Load() { + return ctx, func() context.Context { return ctx }, fmt.Errorf("locker is closed") + } + originalCtx := ctx options := []redsync.Option{ @@ -102,6 +118,10 @@ func (l *redisLocker) lock(ctx context.Context, key string, tries int) (context. } func (l *redisLocker) startExtend() { + if l.closed.Load() { + return + } + toExtend := make([]*redisMutex, 0) l.mutexM.Range(func(_, value any) bool { m := value.(*redisMutex) From a02f8098e13bf741cc08702465d758656c163e3e Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Aug 2024 17:58:57 +0800 Subject: [PATCH 16/19] test: avoid data race --- modules/globallock/locker_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/modules/globallock/locker_test.go b/modules/globallock/locker_test.go index 15a3c65bb070f..2f5861b3793e6 100644 --- a/modules/globallock/locker_test.go +++ b/modules/globallock/locker_test.go @@ -29,6 +29,11 @@ func TestLocker(t *testing.T) { oldExpiry := redisLockExpiry redisLockExpiry = 5 * time.Second // make it shorter for testing defer func() { + // Avoid data race. + // The startExtend goroutine may still be running and reading redisLockExpiry. + // Wait for a while since it will be stopped soon after Close is called. + time.Sleep(time.Second) + redisLockExpiry = oldExpiry }() From c6300df18a4c0ef634c4d27424aa7f5027f41ab3 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Aug 2024 18:20:15 +0800 Subject: [PATCH 17/19] test: wait close avoid data race --- modules/globallock/locker_test.go | 5 ----- modules/globallock/redis_locker.go | 10 ++++++++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/modules/globallock/locker_test.go b/modules/globallock/locker_test.go index 2f5861b3793e6..15a3c65bb070f 100644 --- a/modules/globallock/locker_test.go +++ b/modules/globallock/locker_test.go @@ -29,11 +29,6 @@ func TestLocker(t *testing.T) { oldExpiry := redisLockExpiry redisLockExpiry = 5 * time.Second // make it shorter for testing defer func() { - // Avoid data race. - // The startExtend goroutine may still be running and reading redisLockExpiry. - // Wait for a while since it will be stopped soon after Close is called. - time.Sleep(time.Second) - redisLockExpiry = oldExpiry }() diff --git a/modules/globallock/redis_locker.go b/modules/globallock/redis_locker.go index 89c13fd97c7fe..34b2fabfb393d 100644 --- a/modules/globallock/redis_locker.go +++ b/modules/globallock/redis_locker.go @@ -26,8 +26,9 @@ var redisLockExpiry = 30 * time.Second type redisLocker struct { rs *redsync.Redsync - mutexM sync.Map - closed atomic.Bool + mutexM sync.Map + closed atomic.Bool + extendWg sync.WaitGroup } var _ Locker = &redisLocker{} @@ -40,6 +41,8 @@ func NewRedisLocker(connection string) Locker { ), ), } + + l.extendWg.Add(1) l.startExtend() return l @@ -66,8 +69,10 @@ func (l *redisLocker) TryLock(ctx context.Context, key string) (bool, context.Co // It will stop extending the locks and refuse to acquire new locks. // In actual use, it is not necessary to call this function. // But it's useful in tests to release resources. +// It could take some time since it waits for the extending goroutine to finish. func (l *redisLocker) Close() error { l.closed.Store(true) + l.extendWg.Wait() return nil } @@ -119,6 +124,7 @@ func (l *redisLocker) lock(ctx context.Context, key string, tries int) (context. func (l *redisLocker) startExtend() { if l.closed.Load() { + l.extendWg.Done() return } From da3f5a95125adcf96d453fb929cf3faa656b15aa Mon Sep 17 00:00:00 2001 From: Jason Song Date: Sat, 24 Aug 2024 08:29:49 +0800 Subject: [PATCH 18/19] test: fix --- modules/globallock/globallock_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/modules/globallock/globallock_test.go b/modules/globallock/globallock_test.go index fd4c3323c1c74..8dcfb915030fc 100644 --- a/modules/globallock/globallock_test.go +++ b/modules/globallock/globallock_test.go @@ -26,8 +26,12 @@ func TestLockAndDo(t *testing.T) { } oldDefaultLocker := defaultLocker + oldInitOnce := initOnce + oldInitFunc := initFunc defer func() { defaultLocker = oldDefaultLocker + initOnce = oldInitOnce + initFunc = oldInitFunc }() initOnce = sync.Once{} @@ -40,8 +44,12 @@ func TestLockAndDo(t *testing.T) { }) t.Run("memory", func(t *testing.T) { oldDefaultLocker := defaultLocker + oldInitOnce := initOnce + oldInitFunc := initFunc defer func() { defaultLocker = oldDefaultLocker + initOnce = oldInitOnce + initFunc = oldInitFunc }() initOnce = sync.Once{} From 57cb17ad322e13979455f13964523a1d4c780b37 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Sat, 24 Aug 2024 08:50:29 +0800 Subject: [PATCH 19/19] test: fix reset initOnce --- modules/globallock/globallock_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/modules/globallock/globallock_test.go b/modules/globallock/globallock_test.go index 8dcfb915030fc..88a555c86f3bd 100644 --- a/modules/globallock/globallock_test.go +++ b/modules/globallock/globallock_test.go @@ -26,12 +26,13 @@ func TestLockAndDo(t *testing.T) { } oldDefaultLocker := defaultLocker - oldInitOnce := initOnce oldInitFunc := initFunc defer func() { defaultLocker = oldDefaultLocker - initOnce = oldInitOnce initFunc = oldInitFunc + if defaultLocker == nil { + initOnce = sync.Once{} + } }() initOnce = sync.Once{} @@ -44,12 +45,13 @@ func TestLockAndDo(t *testing.T) { }) t.Run("memory", func(t *testing.T) { oldDefaultLocker := defaultLocker - oldInitOnce := initOnce oldInitFunc := initFunc defer func() { defaultLocker = oldDefaultLocker - initOnce = oldInitOnce initFunc = oldInitFunc + if defaultLocker == nil { + initOnce = sync.Once{} + } }() initOnce = sync.Once{}