After a server Handler Hijacks and returns the underlying net.Conn, I'd expect the Handler's Request.Context to be Done when that Conn is closed.

We've never documented that guarantee. In fact, I believe we document something close to the opposite: that once you call Hijack, the net/http package is no longer involved at all, which includes doing magic context cancellations behind your back.

Let us know if there's missing or inconsistent documentation, but I don't think we can change behavior here.

/cc @tombergan

After a server Handler Hijacks and returns the underlying net.Conn, I'd expect the Handler's Request.Context to be Done when that Conn is closed.

We've never documented that guarantee.

I think we have, actually:

https://golang.org/pkg/net/http/#Request.Context

For incoming server requests, the context is canceled when the client's connection closes, the request is canceled (with HTTP/2), or when the ServeHTTP method returns.

There's no stated exception for hijacked connections so it's reasonable to assume that the above guarantee applies to hijacked connections as well. This would be simple to implement. In conn.hijackLocked, we'd wrap rwc to call cancelCtx() from Close or on error from Read/Write.

The problem is there are users who call Hijack and then typecast the returned net.Conn to *net.TCPConn or *tls.Conn. We haven't documented that this typecast works, but it has worked since the beginning (most likely) and users are relying on it. Wrapping rwc would break those typecasts. Looking through internal google source code, I found at least one major user of Hijack that relies on being able to cast to *net.TCPConn. Outside google code, I notice this. I haven't done a very thorough search ... I'm sure there are other casts like these.

So, we either wrap rwc and break users, or we keep the code as-is and update the documentation. Given the potential breakage, I favor updating the documentation.

If a user wants to cancel req.Context when the connection closes, it's not hard to wrap the returned net.Conn as I described above. Something like:

ctx, cancelCtx := context.WithCancel(req.Context())
req = req.WithContex(ctx)
conn, buff, err := rw.Hijack()
conn = connCancelOnClose{conn, cancelCtx}

type connCancelOnClose struct {  // implements wrappers for Read, Write, Close
  net.Conn
  cancelCtx func()
}

In conn.hijackLocked, we'd wrap rwc to call cancelCtx() from Close or on error from Read/Write.

We can't change the concrete type of the net.Conn returned by Hijack, if that's what you're proposing.

We can't change the concrete type of the net.Conn returned by Hijack, if that's what you're proposing.

I proposed that as a straw man, then argued that it's a bad idea. I don't think we can do anything except clarify the documentation for Request.Context().

Sorry, once again I failed at reading. Yeah, documentation SGTM.

I appreciate clarification that closing the hijacked connection does not cancel the request context. It's a shame this can't be made the case, I think it's another example of the interface type assertion issue that exists elsewhere. It also seems presumptuous of existing users to assume that the net.Conn interface guarantees a specific type, that's why it's an interface in the first place. Clearly here we want to give the hijacker access to the true connection type with everything it implies (for instance TCPConn), but we need to modify a bunch of the methods. I also think we're unable to widen the conn interface passed to the Hijack method because it will cause compile issues. That wouldn't be a problem if functions automatically allowed interface narrowing (so that existing hijackers would just check that the interface methods they require are present and pass type checking).

Change https://golang.org/cl/115039 mentions this issue: net/http: document how Hijack and Request.Context interact

I don't believe the Request Context is canceled when Hijack is called. The new documentation seems to suggest this.

@anacrolix, yeah, thanks for catching that. The docs are wrong. I'll try again.

Change https://golang.org/cl/119235 mentions this issue: net/http: document how Hijack and Request.Context interact, take two