decred / dcrpool

decred mining pool
ISC License
29 stars 27 forks source link

pool: Correct endpoint waitgroup logic. #359

Closed davecgh closed 1 year ago

davecgh commented 1 year ago

This requires #358.

This corrects the waitgroup logic in endpoint handling to ensure it properly terminates when the context is canceled.

It also enables the wait at the end of the endpoint test to help prove correctness.

davecgh commented 1 year ago

There's an issue here which results in dcrpool never shutting down.

  1. connect is in the process of creating a new client
  2. ctx cancelled
  3. disconnect calls cancel on every client in the list
  4. connect adds new client to the list, it is never canceled.

I triggered this condition by adding a time.Sleep immediately after the call to NewClient and CTRL+C'ing dcrpool whilst the sleep was running.

I suspect it's something else, because the new client is created with the context that is already canceled, so it's already canceled.

davecgh commented 1 year ago

For reference later when anyone else is reviewing these PRs. We tracked the issue down offline and @jholdstock submitted #367 to resolve the actual root cause of what he was seeing.

jholdstock commented 1 year ago

Retested with the changes from #367 and this is working well now.