recipes

recipes copied to clipboard

Question description I'm about to implement something similar to the websocket-chat example in one of my projects and the following question popped into my mind: Shouldn't the websocket send the messages to each client in parallel? As far as I understand connection.WriteMessage() is a blocking operation. Doesn't that mean, that the hub blocks until each client receives the broadcasting message? If I'm not mistaken, this can be an issue on clients with a slow connection and a limitation to the hubs processing speed in general.

The following snipped sends each message in a separate gorountine. If writing to the socket connection returns an error, the connection gets closed and send the connection back to the hub to unregister the client.

Code snippet (optional)

type client struct {
	isClosing bool
	mu        sync.Mutex
}

var clients = make(map[*websocket.Conn]*client)
var register = make(chan *websocket.Conn)
var broadcast = make(chan string)
var unregister = make(chan *websocket.Conn)

func runHub() {
	for {
		select {
		case connection := <-register:
			clients[connection] = &client{}
			log.Println("connection registered")

		case message := <-broadcast:
			log.Println("message received:", message)
			// Send the message to all clients
			for connection, c := range clients {
				go func(connection *websocket.Conn, c *client) { // send to each client in parallel so we don't block on a slow client
					c.mu.Lock()
					defer c.mu.Unlock()
					if c.isClosing {
						return
					}
					if err := connection.WriteMessage(websocket.TextMessage, []byte(message)); err != nil {
						c.isClosing = true
						log.Println("write error:", err)

						connection.WriteMessage(websocket.CloseMessage, []byte{})
						connection.Close()
						unregister <- connection
					}
				}(connection, c)
			}

		case connection := <-unregister:
			// Remove the client from the hub
			delete(clients, connection)

			log.Println("connection unregistered")
		}
	}
}

I'm happy to open a PR for this, but I thought I'd ask first if it even makes sense.

Edit: I realized that my initial code snipped caused a race condition and possibly a panic due to concurrent write to websocket connection. Therefore I added a mutex to the client to (hopefully) prevent that.

Thanks for opening your first issue here! 🎉 Be sure to follow the issue template!

@jon4hz Makes sense to me, send a PR for it.

👋 Hello. Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@jon4hz Do you plan on submitting a PR or want me to make the changes in a PR?

Hi, sorry for the late response. I opened #629

Hi, sorry for the late response. I opened #629

Thank you!