Skip to content

fix: add 5s timeout to graceful-shutdown calls that used WithoutCancel#3370

Merged
dgageot merged 1 commit into
docker:mainfrom
dgageot:worktree-board-ae689a7cbfc7e11c
Jul 1, 2026
Merged

fix: add 5s timeout to graceful-shutdown calls that used WithoutCancel#3370
dgageot merged 1 commit into
docker:mainfrom
dgageot:worktree-board-ae689a7cbfc7e11c

Conversation

@dgageot

@dgageot dgageot commented Jul 1, 2026

Copy link
Copy Markdown
Member

Two shutdown paths in the codebase used context.WithoutCancel without pairing it with a deadline, meaning they could block indefinitely if the underlying server or provider failed to stop. In pkg/mcp/server.go, the HTTP server shutdown triggered by a cancelled context had no timeout. In cmd/root/otel.go, the metric-provider shutdown inside a logger-provider error path had the same issue.

Both calls now use a 5-second timeout context, matching the pattern already in place in pkg/chatserver/server.go and in shutdownTracerProvider within the same otel.go file. The fix is purely additive — no behavior changes except that these two paths now bound their wait time the same way all sibling shutdown calls do.

No breaking changes. Validated with task build and task lint (zero new issues).

@dgageot dgageot requested a review from a team as a code owner July 1, 2026 12:03

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

The fix correctly adds 5-second timeout contexts to two previously unbounded context.WithoutCancel shutdown paths.

cmd/root/otel.gocontext.WithTimeout(context.WithoutCancel(ctx), 5*time.Second) is used for mp.Shutdown, with defer cancel() immediately following. The defer fires when initOTelSDK returns (right after the shutdown call), so no context is leaked.

pkg/mcp/server.go — Same pattern applied to httpServer.Shutdown in the case <-ctx.Done(): branch. defer cancel() fires when StartHTTPServer returns, which happens immediately after — correct.

Both changes are consistent with the existing 5-second timeout pattern in pkg/chatserver/server.go and shutdownTracerProvider in the same otel.go file. No bugs introduced.

@dgageot dgageot merged commit 48b9cea into docker:main Jul 1, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants