Sending message in its own goroutine can result in wrong message order.

[FZambia]

Hello! It seems that sending message in its own goroutine:

        case m := <-h.broadcast:
            for s := range h.sessions {
                if m.filter != nil {
                    if m.filter(s) {
                        go s.writeMessage(m)
                    }
                } else {
                    go s.writeMessage(m)
                }
            }

can theoretically result in wrong message order when message rate is high as code relies on the order of goroutine execution.

Maybe I miss something or it's not an issue for melody – just noticed and decided to ask.

[olahol]

You are right, the reason this was done was because I did not wan't to block the hub if the Session.output channel was full.

I have to think about how to ensure that messages are read/written in order as I also notice that there might be problems with this piece of code:

        if t == websocket.TextMessage {
            go messageHandler(s, message)
        }

Thank you for bringing it up :+1:

[FZambia]

As I am developing https://github.com/centrifugal/centrifugo similar in many aspects I also have the same task:

sending to client: https://github.com/centrifugal/centrifugo/blob/master/libcentrifugo/hubs.go#L300

client's send method implementation: https://github.com/centrifugal/centrifugo/blob/master/libcentrifugo/client.go#L177

So Centrifugo uses separate unlimited size queue for every client. At first stages I used buffered channel with size 256 by default and closed client's connection when that buffer was full and sending into channel blocked. But it behaved bad in case of high message rate as client was disconnected just because it received more than 256 messages at one moment. Now with queue there is a little overhead but predictable behaviour.

I have another problem with dealing with slow clients (lots of allocations in code as I don't have access to SetWriteDeadline method at moment - I use SockJS not just Websockets - but hopefully will be able to improve this in near future). But this is not a problem for Melody as you already use SetWriteDeadline.

So this is one of possible solutions. Maybe you find a better one.

[olahol]

Nice, the Centrifugo project looks really cool.

The queue solutions looks like it should work well for Melody, I'll have to take a closer look at it after work.

Thank you for the help!

[olahol]

Thank you for your help @FZambia, as you can see in my code I took a different route than the infinite queue, instead I decided to dispatch the error handler and if the users wishes they can just increase MessageBufferSize.

[FZambia]

Great! Thanks for your responsiveness:)