Concurrent encoding/decoding

[eyudkin]

I found, that mutex (passing to encoder/decoder) does not work, because it should be passed by reference. For now it looks like:

func NewDecoder(r FrameReader, mtx sync.Mutex) *Decoder {
    return &Decoder{
        r: r,
        mtx: mtx,
    }
}

(should be NewDecoder(r FrameReader, mtx *sync.Mutex)) And at the result, decoder and encoder have different mutexes and it causes bad situations when application tries to emit something in the same moment with the PING request and etc.

Unfortunately, I can not fix it fastly, because just making references to one mutex causes deadlock: DecodeHeader method locks mutex and causes infinite NextReader() cycle and so mutex will not be free until any non-ping message from client (so, it blocks server->client messages).

[googollee]

Which branch? v1.4 or master?

[eyudkin]

1.4

[googollee]

You mean https://github.com/googollee/go-socket.io/blob/v1.4/parser/decoder.go#L29 ?

[eyudkin]

Oops, sorry, I forgot that I made some changes in my fork. This mutex was my fix for some concurrent situations.

[eyudkin]

One of them - its writePacket call in encoder.Encode function.

func (e *Encoder) Encode(h Header, args []interface{}) (err error) {
    var w io.WriteCloser
    w, err = e.w.NextWriter(engineio.TEXT)
    if err != nil {
        return
    }

    var buffers [][]byte
    buffers, err = e.writePacket(w, h, args)

    if err != nil {
        return
    }

    for _, b := range buffers {
        w, err = e.w.NextWriter(engineio.BINARY)
        if err != nil {
            return
        }
        err = e.writeBuffer(w, b)
        if err != nil {
            return
        }
    }
    return
}

So, calls like e.w.NextWriter(engineio.TEXT) works with session (it has upgradeLocker mutex) - and its safe. But writePacket - no, and (as I understood) in some cases it closes writer, when reader works with it.

[eyudkin]

So, @googollee - issue is still actual (as for me) ;)

[googollee]

Encoder.writePacket() should not race with eath other. NextWriter() will block each other and only one return writer to do writePacket(). Could you give an example to show the issue?

[eyudkin]

Code screenshot Stacktraces screenshot As I understand the situation: serveWrite->encode->writePacket closes connection without any mutexes (WriteCloser interface), but serveRead ->DecodeHeader->... still works with it in other goroutine. So, my error text is: websocket: write closed

[sshaplygin]

@eyudkin Hi Is it actual problem?

[julienmathevet]

I think we have same issue. We will try to make a test to reproduce it. It seems that appears when we broadcast to many events in short delay with at least one subscriber