Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SetReadDeadline (an alternative to deadline contexts) #252

Open
importnil opened this issue Jun 13, 2020 · 5 comments
Open

SetReadDeadline (an alternative to deadline contexts) #252

importnil opened this issue Jun 13, 2020 · 5 comments
Milestone

Comments

@importnil
Copy link

importnil commented Jun 13, 2020

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?

@nhooyr
Copy link
Owner

nhooyr commented Jun 13, 2020

Already implemented! Check out websocket.NetConn!

@nhooyr nhooyr closed this as completed Jun 13, 2020
@aep
Copy link

aep commented Mar 15, 2022

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 {
		// ...
    }
}

@nhooyr
Copy link
Owner

nhooyr commented Oct 20, 2022

@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
Copy link
Owner

nhooyr commented Mar 6, 2023

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

@nhooyr nhooyr reopened this Mar 6, 2023
@nhooyr
Copy link
Owner

nhooyr commented Sep 28, 2023

Will roll into #402

@nhooyr nhooyr added this to the v2.0.0 milestone Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants