ipfs / ipfs/go-graphsync

Revisit Shutdown Behavior

Open
#220 0 comments 1 reaction 0 assignees View on GitHub
effort/days exp/expert need/maintainer-input need/triage needs definition P2
Dominant language
Go
Stars
103
Forks
41
PR merge metrics
No merged PRs in 30d

Description

We have pretty terrible patterns for shutting down all over the code base.

Currently, you pass a context to Graphsync when you start it and the only way to shutdown is just cancel the context and hope all the go routines get cleaned up (I think they do but...)

Meanwhile, we actually have Shutdown methods for lots of the components but all they do is shut down an internal context... and they're not even exposed publicly.

Per @mvdan :

```
If we went with the "we'll eventually want to block until all request goroutines are stopped" route,
then I think these methods should gain a context parameter, too. That way, I can say "graceful
shutdown for 10s to stop accepting requests, and after that, kill all requests which aren't done
yet". This is how net/http does it: https://pkg.go.dev/net/http#Server.Shutdown
```

We should... have a way to shutdown that confirms all the go routines are actually closed.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.