websocket icon indicating copy to clipboard operation
websocket copied to clipboard

SetReadDeadline (an alternative to deadline contexts)

Open importnil opened this issue 4 years ago • 5 comments

Hey,

For the continuous reads, it's convenient to use something like Gorilla's SetReadDeadline, instead of creating a new context each time a Read is called. Though I know Gorilla's encapsulating the net.Conn, while this pkg isn't. How do you think is it possible to add something like that in this library?

importnil avatar Jun 13 '20 20:06 importnil

Already implemented! Check out websocket.NetConn!

nhooyr avatar Jun 13 '20 21:06 nhooyr

hmm but that creates a new conn instead of letting you cheaply set a deadline on the existing socket.

creating a new context in a loop like this seems like a terrible idea, since the defer isnt called on loop re-entry

for ;;  {
    ctx, cancel := context.WithTimeout(r.Context(), time.Second*10)
    defer cancel()

    var v interface{}
    err = wsjson.Read(ctx, c, &v)
    if err != nil {
		// ...
    }
}

aep avatar Mar 15 '22 10:03 aep

@aep You don't have to create a new net.Conn on every read/write, just once. If the tiny allocation overhead is a real concern, using Go is likely not appropriate. You'd want your websocket server in C.

nhooyr avatar Oct 20 '22 19:10 nhooyr

I'm going to reopen this as I might change the API to a tiered approach where contexts are only used if requested.

nhooyr avatar Mar 06 '23 18:03 nhooyr

Will roll into #402

nhooyr avatar Sep 28 '23 07:09 nhooyr