Skip to content

Commit

Permalink
Add guidance on goroutine lifecycle management (#158)
Browse files Browse the repository at this point in the history
In general, we prefer for goroutines to have well-managed lifecycles.
No uncontrolled background work that cannot be stopped.
This change tries to distill some of the guidance around it into a style
guide entry.
In particular,

- goroutines must end or be stoppable
- APIs must block for goroutines to finish
- `init()` should never spawn a goroutine
  • Loading branch information
abhinav authored Oct 19, 2022
1 parent 7e4a675 commit 4478e67
Show file tree
Hide file tree
Showing 2 changed files with 203 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 2022-10-18

- Add guidance on managing goroutine lifecycles.

# 2022-03-30

- Add guidance on using field tags in marshaled structs.
Expand Down
199 changes: 199 additions & 0 deletions style.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ row before the </tbody></table> line.
- [Exit in Main](#exit-in-main)
- [Exit Once](#exit-once)
- [Use field tags in marshaled structs](#use-field-tags-in-marshaled-structs)
- [Don't fire-and-forget goroutines](#dont-fire-and-forget-goroutines)
- [Wait for goroutines to exit](#wait-for-goroutines-to-exit)
- [No goroutines in `init()`](#no-goroutines-in-init)
- [Performance](#performance)
- [Prefer strconv over fmt](#prefer-strconv-over-fmt)
- [Avoid string-to-byte conversion](#avoid-string-to-byte-conversion)
Expand Down Expand Up @@ -1894,6 +1897,202 @@ this contract. Specifying field names inside tags makes the contract explicit,
and it guards against accidentally breaking the contract by refactoring or
renaming fields.

### Don't fire-and-forget goroutines

Goroutines are lightweight, but they're not free:
at minimum, they cost memory for their stack and CPU to be scheduled.
While these costs are small for typical uses of goroutines,
they can cause significant performance issues
when spawned in large numbers without controlled lifetimes.
Goroutines with unmanaged lifetimes can also cause other issues
like preventing unused objects from being garbage collected
and holding onto resources that are otherwise no longer used.

Therefore, do not leak goroutines in production code.
Use [go.uber.org/goleak](https://pkg.go.dev/go.uber.org/goleak)
to test for goroutine leaks inside packages that may spawn goroutines.

In general, every goroutine:

- must have a predictable time at which it will stop running; or
- there must be a way to signal to the goroutine that it should stop

In both cases, there must be a way code to block and wait for the goroutine to
finish.

For example:

<table>
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
<tbody>
<tr><td>

```go
go func() {
for {
flush()
time.Sleep(delay)
}
}()
```

</td><td>

```go
var (
stop = make(chan struct{}) // tells the goroutine to stop
done = make(chan struct{}) // tells us that the goroutine exited
)
go func() {
defer close(done)

ticker := time.NewTicker(delay)
defer ticker.Stop()
for {
select {
case <-tick.C:
flush()
case <-stop:
return
}
}
}()

// Elsewhere...
close(stop) // signal the goroutine to stop
<-done // and wait for it to exit
```

</td></tr>
<tr><td>

There's no way to stop this goroutine.
This will run until the application exits.

</td><td>

This goroutine can be stopped with `close(stop)`,
and we can wait for it to exit with `<-done`.

</td></tr>
</tbody></table>

#### Wait for goroutines to exit

Given a goroutine spawned by the sytem,
there must be a way to wait for the goroutine to exit.
There are two popular ways to do this:

- Use a `sync.WaitGroup`.
Do this if there are multiple goroutines that you want to wait for

```go
var wg sync.WaitGroup
for i := 0; i < N; i++ {
wg.Add(1)
go func() {
defer wg.Done()
// ...
}()
}

// To wait for all to finish:
wg.Wait()
```

- Add another `chan struct{}` that the goroutine closes when it's done.
Do this if there's only one goroutine.

```go
done := make(chan struct{})
go func() {
defer close(done)
// ...
}()
// To wait for the goroutine to finish:
<-done
```

#### No goroutines in `init()`

`init()` functions should not spawn goroutines.
See also [Avoid init()](#avoid-init).

If a package has need of a background goroutine,
it must expose an object that is responsible for managing a goroutine's
lifetime.
The object must provide a method (`Close`, `Stop`, `Shutdown`, etc)
that signals the background goroutine to stop, and waits for it to exit.
<table>
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
<tbody>
<tr><td>
```go
func init() {
go doWork()
}
func doWork() {
for {
// ...
}
}
```
</td><td>
```go
type Worker struct{ /* ... */ }
func NewWorker(...) *Worker {
w := &Worker{
stop: make(chan struct{}),
done: make(chan struct{}),
// ...
}
go w.doWork()
}
func (w *Worker) doWork() {
defer close(w.done)
for {
// ...
case <-w.stop:
return
}
}
// Shutdown tells the worker to stop
// and waits until it has finished.
func (w *Worker) Shutdown() {
close(w.stop)
<-w.done
}
```
</td></tr>
<tr><td>
Spawns a background goroutine unconditionally when the user exports this package.
The user has no control over the goroutine or a means of stopping it.
</td><td>
Spawns the worker only if the user requests it.
Provides a means of shutting down the worker so that the user can free up
resources used by the worker.
Note that you should use `WaitGroup`s if the worker manages multiple
goroutines.
See [Wait for goroutines to exit](#wait-for-goroutines-to-exit).
</td></tr>
</tbody></table>
## Performance
Performance-specific guidelines apply only to the hot path.
Expand Down

0 comments on commit 4478e67

Please sign in to comment.