Skip to content

Commit c4c1706

Browse files
committed
Merge remote-tracking branch 'giteaofficial/main'
* giteaofficial/main: [skip ci] Updated translations via Crowdin Update JS and PY dependencies (go-gitea#31940) Fix search team (go-gitea#31923) Upgrade micromatch to 4.0.8 (go-gitea#31939) Refactor globallock (go-gitea#31933)
2 parents c160619 + 1f924d8 commit c4c1706

File tree

11 files changed

+6395
-3174
lines changed

11 files changed

+6395
-3174
lines changed

modules/globallock/globallock.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,20 @@ func DefaultLocker() Locker {
2727

2828
// Lock tries to acquire a lock for the given key, it uses the default locker.
2929
// Read the documentation of Locker.Lock for more information about the behavior.
30-
func Lock(ctx context.Context, key string) (context.Context, ReleaseFunc, error) {
30+
func Lock(ctx context.Context, key string) (ReleaseFunc, error) {
3131
return DefaultLocker().Lock(ctx, key)
3232
}
3333

3434
// TryLock tries to acquire a lock for the given key, it uses the default locker.
3535
// Read the documentation of Locker.TryLock for more information about the behavior.
36-
func TryLock(ctx context.Context, key string) (bool, context.Context, ReleaseFunc, error) {
36+
func TryLock(ctx context.Context, key string) (bool, ReleaseFunc, error) {
3737
return DefaultLocker().TryLock(ctx, key)
3838
}
3939

4040
// LockAndDo tries to acquire a lock for the given key and then calls the given function.
4141
// It uses the default locker, and it will return an error if failed to acquire the lock.
4242
func LockAndDo(ctx context.Context, key string, f func(context.Context) error) error {
43-
ctx, release, err := Lock(ctx, key)
43+
release, err := Lock(ctx, key)
4444
if err != nil {
4545
return err
4646
}
@@ -52,7 +52,7 @@ func LockAndDo(ctx context.Context, key string, f func(context.Context) error) e
5252
// TryLockAndDo tries to acquire a lock for the given key and then calls the given function.
5353
// It uses the default locker, and it will return false if failed to acquire the lock.
5454
func TryLockAndDo(ctx context.Context, key string, f func(context.Context) error) (bool, error) {
55-
ok, ctx, release, err := TryLock(ctx, key)
55+
ok, release, err := TryLock(ctx, key)
5656
if err != nil {
5757
return false, err
5858
}

modules/globallock/locker.go

+4-26
Original file line numberDiff line numberDiff line change
@@ -5,56 +5,34 @@ package globallock
55

66
import (
77
"context"
8-
"fmt"
98
)
109

1110
type Locker interface {
1211
// Lock tries to acquire a lock for the given key, it blocks until the lock is acquired or the context is canceled.
1312
//
14-
// Lock returns a new context which should be used in the following code.
15-
// The new context will be canceled when the lock is released or lost - yes, it's possible to lose a lock.
16-
// For example, it lost the connection to the redis server while holding the lock.
17-
// If it fails to acquire the lock, the returned context will be the same as the input context.
18-
//
1913
// Lock returns a ReleaseFunc to release the lock, it cannot be nil.
2014
// It's always safe to call this function even if it fails to acquire the lock, and it will do nothing in that case.
2115
// And it's also safe to call it multiple times, but it will only release the lock once.
2216
// That's why it's called ReleaseFunc, not UnlockFunc.
2317
// But be aware that it's not safe to not call it at all; it could lead to a memory leak.
2418
// So a recommended pattern is to use defer to call it:
25-
// ctx, release, err := locker.Lock(ctx, "key")
26-
// if err != nil {
27-
// return err
28-
// }
29-
// defer release()
30-
// The ReleaseFunc will return the original context which was used to acquire the lock.
31-
// It's useful when you want to continue to do something after releasing the lock.
32-
// At that time, the ctx will be canceled, and you can use the returned context by the ReleaseFunc to continue:
33-
// ctx, release, err := locker.Lock(ctx, "key")
19+
// release, err := locker.Lock(ctx, "key")
3420
// if err != nil {
3521
// return err
3622
// }
3723
// defer release()
38-
// doSomething(ctx)
39-
// ctx = release()
40-
// doSomethingElse(ctx)
41-
// Please ignore it and use `defer release()` instead if you don't need this, to avoid forgetting to release the lock.
4224
//
4325
// Lock returns an error if failed to acquire the lock.
4426
// Be aware that even the context is not canceled, it's still possible to fail to acquire the lock.
4527
// For example, redis is down, or it reached the maximum number of tries.
46-
Lock(ctx context.Context, key string) (context.Context, ReleaseFunc, error)
28+
Lock(ctx context.Context, key string) (ReleaseFunc, error)
4729

4830
// TryLock tries to acquire a lock for the given key, it returns immediately.
4931
// It follows the same pattern as Lock, but it doesn't block.
5032
// And if it fails to acquire the lock because it's already locked, not other reasons like redis is down,
5133
// it will return false without any error.
52-
TryLock(ctx context.Context, key string) (bool, context.Context, ReleaseFunc, error)
34+
TryLock(ctx context.Context, key string) (bool, ReleaseFunc, error)
5335
}
5436

5537
// ReleaseFunc is a function that releases a lock.
56-
// It returns the original context which was used to acquire the lock.
57-
type ReleaseFunc func() context.Context
58-
59-
// ErrLockReleased is used as context cause when a lock is released
60-
var ErrLockReleased = fmt.Errorf("lock released")
38+
type ReleaseFunc func()

modules/globallock/locker_test.go

+20-50
Original file line numberDiff line numberDiff line change
@@ -47,27 +47,24 @@ func TestLocker(t *testing.T) {
4747
func testLocker(t *testing.T, locker Locker) {
4848
t.Run("lock", func(t *testing.T) {
4949
parentCtx := context.Background()
50-
ctx, release, err := locker.Lock(parentCtx, "test")
50+
release, err := locker.Lock(parentCtx, "test")
5151
defer release()
5252

53-
assert.NotEqual(t, parentCtx, ctx) // new context should be returned
5453
assert.NoError(t, err)
5554

5655
func() {
57-
parentCtx, cancel := context.WithTimeout(context.Background(), time.Second)
56+
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
5857
defer cancel()
59-
ctx, release, err := locker.Lock(parentCtx, "test")
58+
release, err := locker.Lock(ctx, "test")
6059
defer release()
6160

6261
assert.Error(t, err)
63-
assert.Equal(t, parentCtx, ctx) // should return the same context
6462
}()
6563

6664
release()
67-
assert.Error(t, ctx.Err())
6865

6966
func() {
70-
_, release, err := locker.Lock(context.Background(), "test")
67+
release, err := locker.Lock(context.Background(), "test")
7168
defer release()
7269

7370
assert.NoError(t, err)
@@ -76,29 +73,26 @@ func testLocker(t *testing.T, locker Locker) {
7673

7774
t.Run("try lock", func(t *testing.T) {
7875
parentCtx := context.Background()
79-
ok, ctx, release, err := locker.TryLock(parentCtx, "test")
76+
ok, release, err := locker.TryLock(parentCtx, "test")
8077
defer release()
8178

8279
assert.True(t, ok)
83-
assert.NotEqual(t, parentCtx, ctx) // new context should be returned
8480
assert.NoError(t, err)
8581

8682
func() {
87-
parentCtx, cancel := context.WithTimeout(context.Background(), time.Second)
83+
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
8884
defer cancel()
89-
ok, ctx, release, err := locker.TryLock(parentCtx, "test")
85+
ok, release, err := locker.TryLock(ctx, "test")
9086
defer release()
9187

9288
assert.False(t, ok)
9389
assert.NoError(t, err)
94-
assert.Equal(t, parentCtx, ctx) // should return the same context
9590
}()
9691

9792
release()
98-
assert.Error(t, ctx.Err())
9993

10094
func() {
101-
ok, _, release, _ := locker.TryLock(context.Background(), "test")
95+
ok, release, _ := locker.TryLock(context.Background(), "test")
10296
defer release()
10397

10498
assert.True(t, ok)
@@ -107,15 +101,15 @@ func testLocker(t *testing.T, locker Locker) {
107101

108102
t.Run("wait and acquired", func(t *testing.T) {
109103
ctx := context.Background()
110-
_, release, err := locker.Lock(ctx, "test")
104+
release, err := locker.Lock(ctx, "test")
111105
require.NoError(t, err)
112106

113107
wg := &sync.WaitGroup{}
114108
wg.Add(1)
115109
go func() {
116110
defer wg.Done()
117111
started := time.Now()
118-
_, release, err := locker.Lock(context.Background(), "test") // should be blocked for seconds
112+
release, err := locker.Lock(context.Background(), "test") // should be blocked for seconds
119113
defer release()
120114
assert.Greater(t, time.Since(started), time.Second)
121115
assert.NoError(t, err)
@@ -127,34 +121,15 @@ func testLocker(t *testing.T, locker Locker) {
127121
wg.Wait()
128122
})
129123

130-
t.Run("continue after release", func(t *testing.T) {
131-
ctx := context.Background()
132-
133-
ctxBeforeLock := ctx
134-
ctx, release, err := locker.Lock(ctx, "test")
135-
136-
require.NoError(t, err)
137-
assert.NoError(t, ctx.Err())
138-
assert.NotEqual(t, ctxBeforeLock, ctx)
139-
140-
ctxBeforeRelease := ctx
141-
ctx = release()
142-
143-
assert.NoError(t, ctx.Err())
144-
assert.Error(t, ctxBeforeRelease.Err())
145-
146-
// so it can continue with ctx to do more work
147-
})
148-
149124
t.Run("multiple release", func(t *testing.T) {
150125
ctx := context.Background()
151126

152-
_, release1, err := locker.Lock(ctx, "test")
127+
release1, err := locker.Lock(ctx, "test")
153128
require.NoError(t, err)
154129

155130
release1()
156131

157-
_, release2, err := locker.Lock(ctx, "test")
132+
release2, err := locker.Lock(ctx, "test")
158133
defer release2()
159134
require.NoError(t, err)
160135

@@ -163,7 +138,7 @@ func testLocker(t *testing.T, locker Locker) {
163138
// and it shouldn't affect the other lock
164139
release1()
165140

166-
ok, _, release3, err := locker.TryLock(ctx, "test")
141+
ok, release3, err := locker.TryLock(ctx, "test")
167142
defer release3()
168143
require.NoError(t, err)
169144
// It should be able to acquire the lock;
@@ -184,28 +159,23 @@ func testRedisLocker(t *testing.T, locker *redisLocker) {
184159
// Otherwise, it will affect other tests.
185160
t.Run("close", func(t *testing.T) {
186161
assert.NoError(t, locker.Close())
187-
_, _, err := locker.Lock(context.Background(), "test")
162+
_, err := locker.Lock(context.Background(), "test")
188163
assert.Error(t, err)
189164
})
190165
}()
191166

192167
t.Run("failed extend", func(t *testing.T) {
193-
ctx, release, err := locker.Lock(context.Background(), "test")
168+
release, err := locker.Lock(context.Background(), "test")
194169
defer release()
195170
require.NoError(t, err)
196171

197172
// It simulates that there are some problems with extending like network issues or redis server down.
198173
v, ok := locker.mutexM.Load("test")
199174
require.True(t, ok)
200-
m := v.(*redisMutex)
201-
_, _ = m.mutex.Unlock() // release it to make it impossible to extend
202-
203-
select {
204-
case <-time.After(redisLockExpiry + time.Second):
205-
t.Errorf("lock should be expired")
206-
case <-ctx.Done():
207-
var errTaken *redsync.ErrTaken
208-
assert.ErrorAs(t, context.Cause(ctx), &errTaken)
209-
}
175+
m := v.(*redsync.Mutex)
176+
_, _ = m.Unlock() // release it to make it impossible to extend
177+
178+
// In current design, callers can't know the lock can't be extended.
179+
// Just keep this case to improve the test coverage.
210180
})
211181
}

modules/globallock/memory_locker.go

+7-20
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,13 @@ func NewMemoryLocker() Locker {
1919
return &memoryLocker{}
2020
}
2121

22-
func (l *memoryLocker) Lock(ctx context.Context, key string) (context.Context, ReleaseFunc, error) {
23-
originalCtx := ctx
24-
22+
func (l *memoryLocker) Lock(ctx context.Context, key string) (ReleaseFunc, error) {
2523
if l.tryLock(key) {
26-
ctx, cancel := context.WithCancelCause(ctx)
2724
releaseOnce := sync.Once{}
28-
return ctx, func() context.Context {
25+
return func() {
2926
releaseOnce.Do(func() {
3027
l.locks.Delete(key)
31-
cancel(ErrLockReleased)
3228
})
33-
return originalCtx
3429
}, nil
3530
}
3631

@@ -39,39 +34,31 @@ func (l *memoryLocker) Lock(ctx context.Context, key string) (context.Context, R
3934
for {
4035
select {
4136
case <-ctx.Done():
42-
return ctx, func() context.Context { return originalCtx }, ctx.Err()
37+
return func() {}, ctx.Err()
4338
case <-ticker.C:
4439
if l.tryLock(key) {
45-
ctx, cancel := context.WithCancelCause(ctx)
4640
releaseOnce := sync.Once{}
47-
return ctx, func() context.Context {
41+
return func() {
4842
releaseOnce.Do(func() {
4943
l.locks.Delete(key)
50-
cancel(ErrLockReleased)
5144
})
52-
return originalCtx
5345
}, nil
5446
}
5547
}
5648
}
5749
}
5850

59-
func (l *memoryLocker) TryLock(ctx context.Context, key string) (bool, context.Context, ReleaseFunc, error) {
60-
originalCtx := ctx
61-
51+
func (l *memoryLocker) TryLock(_ context.Context, key string) (bool, ReleaseFunc, error) {
6252
if l.tryLock(key) {
63-
ctx, cancel := context.WithCancelCause(ctx)
6453
releaseOnce := sync.Once{}
65-
return true, ctx, func() context.Context {
54+
return true, func() {
6655
releaseOnce.Do(func() {
67-
cancel(ErrLockReleased)
6856
l.locks.Delete(key)
6957
})
70-
return originalCtx
7158
}, nil
7259
}
7360

74-
return false, ctx, func() context.Context { return originalCtx }, nil
61+
return false, func() {}, nil
7562
}
7663

7764
func (l *memoryLocker) tryLock(key string) bool {

0 commit comments

Comments
 (0)