jsonrpc: Stop background (more) rescans gracefully #2434
Conversation
jholdstock
changed the title
This ensures that the context of in-flight RPC requests is cancelled when the dcrwallet process context is cancelled.
Use the server waitgroup to ensure background rescans can return on their own terms rather than being forcefully terminated early. This is still not an ideal situation because in the case where the server/wallet is being shut down, RescanFromHeight will return with an "rpc connection closed" error (or similar) rather than returning because its provided context was cancelled. However, that is a notable improvement over the existing situation which could result in an unhandled panic.
Using the server context instead of context.Background means that the long-lived rescan funcs are able to exit properly when the server is shutdown and its context is closed.
|
Ah, i think this answers my question from 2435. You are addressing the requests which may spawn a rescan in the background, not the |
| @@ -2056,7 +2056,8 @@ func (s *Server) importPrivKey(ctx context.Context, icmd any) (any, error) { | |||
| s.wg.Add(1) | |||
| go func() { | |||
| defer s.wg.Done() | |||
| _ = w.RescanFromHeight(context.Background(), n, scanFrom) | |||
| serverCtx := s.httpServer.BaseContext(nil) | |||
There was a problem hiding this comment.
We can use the outer context here instead of calling BaseContext ourselves. It will be canceled when the process begins shutdown.
There was a problem hiding this comment.
or well no. let me test that as well. it's possible that this context is canceled after the handler returns....
There was a problem hiding this comment.
By outer context do you mean the one passed into importPrivKey? That will be cancelled as soon as a response is sent to the RPC client, hence why we want to use the server context instead.
There was a problem hiding this comment.
Yep, the rescan immediately exits with that suggested change.
Use the server waitgroup to ensure background rescans can return on their own terms rather than being forcefully terminated early.
Using the server context instead of context.Background means that the long-lived rescan funcs are able to exit properly when the server is shutdown and its context is closed.
Based on #2435