Skip to content

Commit 891e962

Browse files
committed
fn: GoroutineManager.Go returns bool, not error
This was requested in #9140 (comment)
1 parent 669ebf4 commit 891e962

File tree

2 files changed

+15
-17
lines changed

2 files changed

+15
-17
lines changed

fn/goroutine_manager.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,9 @@ package fn
22

33
import (
44
"context"
5-
"errors"
65
"sync"
76
)
87

9-
// ErrStopping is returned when trying to add a new goroutine while stopping.
10-
var ErrStopping = errors.New("can not add goroutine, stopping")
11-
128
// GoroutineManager is used to launch goroutines until context expires or the
139
// manager is stopped. The Stop method blocks until all started goroutines stop.
1410
type GoroutineManager struct {
@@ -29,8 +25,10 @@ func NewGoroutineManager(ctx context.Context) *GoroutineManager {
2925
}
3026
}
3127

32-
// Go starts a new goroutine if the manager is not stopping.
33-
func (g *GoroutineManager) Go(f func(ctx context.Context)) error {
28+
// Go tries to start a new goroutine and returns a boolean indicating its
29+
// success. It fails iff the goroutine manager is stopping or its context passed
30+
// to NewGoroutineManager has expired.
31+
func (g *GoroutineManager) Go(f func(ctx context.Context)) bool {
3432
// Calling wg.Add(1) and wg.Wait() when wg's counter is 0 is a race
3533
// condition, since it is not clear should Wait() block or not. This
3634
// kind of race condition is detected by Go runtime and results in a
@@ -43,7 +41,7 @@ func (g *GoroutineManager) Go(f func(ctx context.Context)) error {
4341
defer g.mu.Unlock()
4442

4543
if g.ctx.Err() != nil {
46-
return ErrStopping
44+
return false
4745
}
4846

4947
g.wg.Add(1)
@@ -52,7 +50,7 @@ func (g *GoroutineManager) Go(f func(ctx context.Context)) error {
5250
f(g.ctx)
5351
}()
5452

55-
return nil
53+
return true
5654
}
5755

5856
// Stop prevents new goroutines from being added and waits for all running
@@ -66,7 +64,7 @@ func (g *GoroutineManager) Stop() {
6664
// safe, since it can't run in parallel with wg.Add(1) call in Go, since
6765
// we just cancelled the context and even if Go call starts running here
6866
// after acquiring the mutex, it would see that the context has expired
69-
// and return ErrStopping instead of calling wg.Add(1).
67+
// and return false instead of calling wg.Add(1).
7068
g.wg.Wait()
7169
}
7270

fn/goroutine_manager_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func TestGoroutineManager(t *testing.T) {
2020

2121
taskChan := make(chan struct{})
2222

23-
require.NoError(t, m.Go(func(ctx context.Context) {
23+
require.True(t, m.Go(func(ctx context.Context) {
2424
<-taskChan
2525
}))
2626

@@ -38,7 +38,7 @@ func TestGoroutineManager(t *testing.T) {
3838
require.Greater(t, stopDelay, time.Second)
3939

4040
// Make sure new goroutines do not start after Stop.
41-
require.ErrorIs(t, m.Go(func(ctx context.Context) {}), ErrStopping)
41+
require.False(t, m.Go(func(ctx context.Context) {}))
4242

4343
// When Stop() is called, the internal context expires and m.Done() is
4444
// closed. Test this.
@@ -57,7 +57,7 @@ func TestGoroutineManagerContextExpires(t *testing.T) {
5757

5858
m := NewGoroutineManager(ctx)
5959

60-
require.NoError(t, m.Go(func(ctx context.Context) {
60+
require.True(t, m.Go(func(ctx context.Context) {
6161
<-ctx.Done()
6262
}))
6363

@@ -80,7 +80,7 @@ func TestGoroutineManagerContextExpires(t *testing.T) {
8080
}
8181

8282
// Make sure new goroutines do not start after context expiry.
83-
require.ErrorIs(t, m.Go(func(ctx context.Context) {}), ErrStopping)
83+
require.False(t, m.Go(func(ctx context.Context) {}))
8484

8585
// Stop will wait for all goroutines to stop.
8686
m.Stop()
@@ -108,11 +108,11 @@ func TestGoroutineManagerStress(t *testing.T) {
108108
// implementation, this test crashes under `-race`.
109109
for i := 0; i < 100; i++ {
110110
taskChan := make(chan struct{})
111-
err := m.Go(func(ctx context.Context) {
111+
ok := m.Go(func(ctx context.Context) {
112112
close(taskChan)
113113
})
114114
// If goroutine was started, wait for its completion.
115-
if err == nil {
115+
if ok {
116116
<-taskChan
117117
}
118118
}
@@ -134,10 +134,10 @@ func TestGoroutineManagerStopsStress(t *testing.T) {
134134
jobChan := make(chan struct{})
135135

136136
// Start a task and wait inside it until we start calling Stop() method.
137-
err := m.Go(func(ctx context.Context) {
137+
ok := m.Go(func(ctx context.Context) {
138138
<-jobChan
139139
})
140-
require.NoError(t, err)
140+
require.True(t, ok)
141141

142142
// Now launch many gorotines calling Stop() method in parallel.
143143
var wg sync.WaitGroup

0 commit comments

Comments
 (0)