gorilla / websocket

Package gorilla/websocket is a fast, well-tested and widely used WebSocket implementation for Go.
https://gorilla.github.io
BSD 2-Clause "Simplified" License
22.1k stars 3.47k forks source link

SetReadDeadline - Chat example (remove idle clients) #960

Open sofasurfa opened 2 hours ago

sofasurfa commented 2 hours ago

Is there an existing issue for this?

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

hulkingshtick commented 2 hours ago

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.

The code in readPump looks correct. ReadJSON returns with an error when the peer sends a close message, the peer terminates the TCP connection, or when the read deadline is exceeded.

That socket/connection should be deleted for good after ReadDeadline, it's not.

We cannot determine why this is happening because you don't show the code that manages the collection of Client or connections. In any case, that's an application issue and not a websocket issue.

sofasurfa commented 1 hour ago

@hulkingshtick thx for response, my code assumes it's the same as in the chat example.

sofasurfa commented 50 minutes ago

@hulkingshtick but in cases when let's say phone battery dies and websocket close() isn't sent from client, thus read isn't called on server (err not returned from ReadJSON). Then the connection will still be left opened, right? In that case we're back to the problem I was talking about before, which is defer not being called...