Repository navigation
SetReadDeadline - Chat example (remove idle clients) #960
Description
Is there an existing issue for this?
- I have searched the existing issues
Current Behavior
I used chat example to modify some code, added clients map and send chan. The example makes sense that after pongWait the next reads return err and cause defer to be called thus calling c.hub.unregister <- c. Happy days.
**hub.go**
---
type Hub struct {
// Registered clients.
clients map[uuid.UUID]*Client
// Send to specific user
send chan Message
// Inbound messages from the clients.
broadcast chan []byte
// Register requests from the clients.
register chan *Client
// Unregister requests from clients.
unregister chan *Client
}
**client.go**
---
func (c *Client) readPump(env *config.Env) {
defer func() {
c.hub.unregister <- c
c.conn.Close()
}()
c.conn.SetReadLimit(maxMessageSize)
c.conn.SetReadDeadline(time.Now().Add(pongWait))
// Reset pongWait after we get the pong
// to keep client conn opened.
c.conn.SetPongHandler(func(string) error { c.conn.SetReadDeadline(time.Now().Add(pongWait)); return nil })
for {
err := c.conn.ReadJSON(&msg)
if err != nil {
if websocket.IsUnexpectedCloseError(err, websocket.CloseGoingAway, websocket.CloseAbnormalClosure) {
env.SugaredLogger.Error("error in ws ReadJSON:", err)
}
break
}
c.hub.send <- msg
}
}
Expected Behavior
Let's say app user exits, and pong never comes back (thus no next read)... that means the socket is still opened (kind of blocked now but still open)? That socket/connection should be deleted for good after ReadDeadline, it's not. How can I make defer work (that will call c.hub.unregister <- c) after pongWait without waiting for the next read which might not happen. I might have missed something obvious in the code or should have another goroutine checking clients and if they have expired and remove manually? Thanks. Hopefully this will help other developers who are a little confused.
@garyburd
Steps To Reproduce
No response
Anything else?
No response
Activity
[-]SetReadDeadline - Chat example (remove client from list)[/-][+]SetReadDeadline - Chat example (remove idle client from list)[/+][-]SetReadDeadline - Chat example (remove idle client from list)[/-][+]SetReadDeadline - Chat example (remove idle clients)[/+]sofasurfa commentedon Sep 21, 2024
@hulkingshtick thx for response, my code assumes it's the same as in the chat example.
sofasurfa commentedon Sep 21, 2024
@garyburd @hulkingshtick thx, just re-read what you wrote, so you're saying ReadJSON will return err itself without client sending anything? It's just the comment explaining the function says otherwise ("all future reads will return an error"), that why I'm confused I guess:
based on the comment, in case someone's phone battery dies or client network doesn't send a websocket close(), then ReadJSON won't return
errat all even afterReadDeadline.(p.s I didn't look at the pkg code, only the comment my VsCode gave for the function)sofasurfa commentedon Sep 22, 2024
@garyburd @hulkingshtick would appreciate if you could confirm this "in case someone's phone battery dies or client network doesn't send a websocket close(), then ReadJSON won't return err at all even after ReadDeadline bcz a read never came through"
sofasurfa commentedon Sep 22, 2024
@garyburd @hulkingshtick I think I found the code in
conn.go line 1028infunc (c *Conn) NextReader()that returnc.readErrwhich means indeed like you said, ReadJSON will return err after
SetReadDeadline, supported by the comment for this function as wellSo maybe making ReadJSON comment more clear about err being returned after
SetReadDeadlineso it's more clear to avoid confusion? Or maybe updatingnextReader()comment and adding info aboutSetReadDeadlinesinceReadJSONusesnextReaderanyway?sofasurfa commentedon Sep 22, 2024
@hulkingshtick ahh see it's an internal go pkg doing the work. Thx for clarifying that