Skip to content

Commit

Permalink
Fix strange relationship between local contexts in ...
Browse files Browse the repository at this point in the history
... handlingLoop[Command].run().

This method derives two context ojects mirroring the structure found in
the Server type: `gCtx` for graceful stop and `ctx` for "brute forcing".
Those local context objects must be derived from the stream context, so
they can both be used for logging.

They were arrenged in such a way that the local `gCtx` is child of
`ctx`. If the server own `ctx` is cancelled to force a stop, depending
on the moment that happens it could be interpreted by this method as a
graceful stop (because it only checks `gCtx` and that can be cancelled
due its parent being cancelled). That wouldn't occur if the `gCtx` was
sibling of `ctx` (thus direct child of `h.stream.Context()`), which
should be enough for the logging use case.

The whole point here is that `server.Stop()` is assumed in tests to
cause `server.Serve()` to return a non-nil error. But that can only
happen if the handling loop objects fail or perceive a brute-force
cancellation. If `handlingLooop[Command].run()` perceives `server.Stop()`
as a graceful cancellation, then they won't return errors.

Whether `server.Stop()` implies in a graceful cancellation for the
handling loops was just a matter of in which moment of the for{} the
notification arrived. With the decoupling herein presented, that should
never be possible.
  • Loading branch information
CarlosNihelton committed Aug 19, 2024
1 parent aa9f8ca commit c6af159
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion wsl-pro-service/internal/streams/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func (h *handlingLoop[Command]) run(s *Server, client *multiClient) error {
defer cancel()

// Use this context to log onto the stream, but cancel with server.GracefulStop
gCtx, cancel := cancelWith(ctx, s.gracefulCtx)
gCtx, cancel := cancelWith(h.stream.Context(), s.gracefulCtx)
defer cancel()

for {
Expand Down

0 comments on commit c6af159

Please sign in to comment.