streamable client Close() blocks on DELETE to an unresponsive server (cancel happens after Do)

[whasamatau]

What

streamableClientConn.Close() (mcp/streamable.go) terminates the session with a synchronous HTTP DELETE and calls its own cancel() only after Do() returns. Same ordering on current main (Close ~L2762; c.client.Do(req) ~L2776, c.cancel() ~L2785) and in v1.6.1 / v1.7.0:

func (c *streamableClientConn) Close() error {
    c.closeOnce.Do(func() {
        ...
        req, _ := http.NewRequestWithContext(c.ctx, http.MethodDelete, c.url, nil)
        ... c.client.Do(req)   // synchronous DELETE
        c.cancel()             // only after Do returns
        close(c.done)
    })
    return c.closeErr
}

The DELETE rides c.ctx — the connection's own context, detached from the caller's via xcontext.Detach — and cancel() runs after Do. So a caller cannot bound Close() by cancelling the context it passed in. If the server is reachable at the TCP layer but never responds (black-holed — a host that dropped off the network without sending a RST), and the client's http.Client has no ResponseHeaderTimeout/Timeout, Do() blocks until the OS TCP timeout — minutes, or effectively forever on a reused idle connection. Close() never returns in that window.

Repro

Self-contained; Close() blocks instead of returning promptly:

func TestCloseHangsOnBlackHoledDelete(t *testing.T) {
    server := mcp.NewServer(&mcp.Implementation{Name: "s", Version: "0"}, nil)
    base := mcp.NewStreamableHTTPHandler(func(*http.Request) *mcp.Server { return server }, nil)
    ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        if r.Method == http.MethodDelete {
            <-r.Context().Done() // never respond: mimics a peer that dropped off-net
            return
        }
        base.ServeHTTP(w, r)
    }))
    defer ts.Close()

    client := mcp.NewClient(&mcp.Implementation{Name: "c", Version: "0"}, nil)
    session, err := client.Connect(context.Background(),
        &mcp.StreamableClientTransport{Endpoint: ts.URL}, nil)
    if err != nil {
        t.Fatal(err)
    }

    done := make(chan struct{})
    go func() { session.Close(); close(done) }()
    select {
    case <-done: // want: Close returns promptly
    case <-time.After(5 * time.Second):
        t.Fatal("Close() blocked on a black-holing DELETE")
    }
}

Impact

Any code that closes sessions on teardown — a defer session.Close() at end of session, or a supervisor reloading/stopping a backend — inherits an unbounded stall on a dead-but-not-RSTing peer. We hit this independently in two codebases (an MCP gateway's backend teardown, and an MCP client's session-close defer); both had to wrap Close() in a bounded/detached goroutine, because the caller's cancel can't reach the DELETE.

Suggested fix

Bound the DELETE by construction so callers don't each need a wrapper — e.g. give the DELETE its own short derived context (context.WithTimeout), or cancel/detach around Do() so a hung DELETE can't hold Close() open. Cancelling before (or concurrently with) the Do would also let a caller's context cancellation propagate.

Notes

Distinct from #928 / #929 (which fixed a response-body leak in the same Close, not the blocking behaviour) and from #683 (transient-error connection poisoning on subsequent calls). Confirmed present in v1.6.1, v1.7.0, and current main.

[LPX-E5BD8]

looked at Close() — cancel really is after Do, so if the peer just goes quiet you're stuck until TCP gives up.

I can put a short timeout on that DELETE and add the hang test unless someone's already on it.

[whasamatau]

Thanks for looking — nobody else is on it, so please go ahead. A short timeout (or a dedicated derived context) on the DELETE is exactly the fix I was hoping for.

The repro in the issue is self-contained if you want to drop it straight in as the hang test — it blocks the server's DELETE handler and asserts Close() returns rather than waiting on TCP.