How to ensure concurrency in Golang gorilla WebSocket package

Question

I have studied the Godoc of the gorilla/websocket package.

In the Godoc it is clearly stated that

Concurrency Connections support one concurrent reader and one concurrent writer.

Applications are responsible for ensuring that no more than one goroutine calls the write methods (NextWriter, SetWriteDeadline, WriteMessage, WriteJSON, EnableWriteCompression, SetCompressionLevel) concurrently and that no more than one goroutine calls the read methods (NextReader, SetReadDeadline, ReadMessage, ReadJSON, SetPongHandler, SetPingHandler) concurrently.

The Close and WriteControl methods can be called concurrently with all other methods.

However, in one of the example provided by the package

func (c *Conn) readPump() {
    defer func() {
        hub.unregister <- c
        c.ws.Close()
    }()
    c.ws.SetReadLimit(maxMessageSize)
    c.ws.SetReadDeadline(time.Now().Add(pongWait))
    c.ws.SetPongHandler(func(string) error { 
        c.ws.SetReadDeadline(time.Now().Add(pongWait)); return nil
    })
    for {
        _, message, err := c.ws.ReadMessage()
        if err != nil {
            if websocket.IsUnexpectedCloseError(err, websocket.CloseGoingAway) {
                log.Printf("error: %v", err)
            }
            break
        }
        message = bytes.TrimSpace(bytes.Replace(message, newline, space, -1))
        hub.broadcast <- message
    }
}

Source: https://github.com/gorilla/websocket/blob/a68708917c6a4f06314ab4e52493cc61359c9d42/examples/chat/conn.go#L50

This line

c.ws.SetPongHandler(func(string) error { 
    c.ws.SetReadDeadline(time.Now().Add(pongWait)); return nil
})

and this line

_, message, err := c.ws.ReadMessage()

seems to be not synchronized because the first line is a callback function so it should be invoked in a Goroutine created in the package and the second line is executing in the Goroutine that invoke serveWs

More importantly, how should I ensure that no more than one goroutine calls the SetReadDeadline, ReadMessage, SetPongHandler, SetPingHandler concurrently?

I tries to use a Mutex lock and lock it whenever I call the above functions, and unlock it afterwards, but quickly I realize a problem. It is usual (also in the example) that ReadMessage is being called in a for-loop. But if the Mutext is locked before the ReadMessage, then no other Read-functions can acquire the lock and execute until next message is received

Is there any better way in handling this concurrency issue? Thanks in advance.

Tapley

After studying inside the package, if I didn't misread it, the ping and pong callback functions are actually executing in the same GoRoutine as the ReadMessage() is called. So the example still guarantees the concurrency. And SetPingHandler() and SetPongHandler() they also need to be synchronized, but in the example they are called before any ReadMessage() is called, so it is ok. – Tapley

Answer 1

The best way to ensure that there are no concurrent calls to the read methods is to execute all of the read methods from a single goroutine.

All of the Gorilla websocket examples use this approach including the example pasted in the question. In the example, all calls to the read methods are from the readPump method. The readPump method is called once for a connection on a single goroutine. It follows that the connection read methods are not called concurrently.

The section of the documentation on control messages says that the application must read the connection to process control messages. Based on this and Gorilla's own examples, I think it's safe to assume that the ping, pong and close handlers will be called from the application's reading goroutine as it is in the current implementation. It would be nice if the documentation could be more explicit about this. Maybe file an issue?

Wapentake