jaspervdj / websockets

A Haskell library for creating WebSocket-capable servers
http://jaspervdj.be/websockets
BSD 3-Clause "New" or "Revised" License
407 stars 113 forks source link

can multiple threads send at the same time? #208

Open dktr0 opened 4 years ago

dktr0 commented 4 years ago

Possibly a simple question but I'm having a hard time finding an answer for it: can 'send' be called with the same Connection from different threads concurrently? (Seeing a bug in my application that would be explained if the answer was 'no, not safely' but also can't find anywhere where it says this is safe or not safe...) Thanks!

ondrap commented 3 years ago

I had the same problem and IMO the answer is probably 'no'. Which frightens me a little as e.g. the receiveData function can also send messages. However, given that I am using per-message-deflate and the messages did arrive, it seems to me that the problem might be somewhere else - I would have expected a decompression error, not intermixed messages.

ondrap commented 3 years ago

So after searching a little I suspect this to be a problem:

makeMessageDeflater (Just pmd)
    | serverNoContextTakeover pmd = do
        return $ \msg -> do
            ptr <- initDeflate pmd
            deflateMessageWith (deflateBody ptr) msg
    | otherwise = do
        ptr <- initDeflate pmd
        return $ \msg ->
            deflateMessageWith (deflateBody ptr) msg

Unless the connection is with serverNoContextTakeover, this is very likely thread unsafe. I do not easily see how this may be made thread-safe though; the messages must be sent in the exact order in which they were compressed, so the only way seems to me would be to do a wrapper around write in Network.WebSockets.Connection/acceptRequestWith. It seems to be safe with control messages, so it is probably possible to serialize this in the app and it shouldn't cause any problems.

dktr0 commented 3 years ago

Thanks! In the months that have passed since I originally posted this issue, I refactored my application to not send from multiple threads and the problem seems to have gone away. Mentioning as further "evidence" in case it helps anyone else.

kabuhr commented 2 years ago

@dktr0, can you confirm that you were using PermessageDeflateCompression in your app as well, when you noticed the issue?

dktr0 commented 2 years ago

@kabuhr I'm not sure but I think I would have been using whatever the default connections are?

qwbarch commented 2 years ago

Hmm so it's not from compression then? Since the default connection options doesn't have it enabled

@dktr0 How were you able to refactor your application to send messages from a single thread, when receiveData also sends messages (on pings and close)? I asked about this on stack overflow as well

Edit: Nevermind, took a look at your repository for inspiration. I guess it's not too much of a worry about receiveData sending messages from other threads then

dktr0 commented 2 years ago

@qwbarch it might be a worry though still? I will say that we still do experience highly intermittent/refractory problems where a websocket connection seems to die (from the client's point of view) without ever dying from the server's point of view, and now this renewed conversation is making me wonder if it doesn't have something to do with this still...

domenkozar commented 2 years ago

@qwbarch it might be a worry though still? I will say that we still do experience highly intermittent/refractory problems where a websocket connection seems to die (from the client's point of view) without ever dying from the server's point of view, and now this renewed conversation is making me wonder if it doesn't have something to do with this still...

I'm seeing that too, did you ever manage to narrow it down?

dktr0 commented 2 years ago

@domenkozar not yet - still on the TODO list...

domenkozar commented 2 years ago

@dktr0 do you have some notes on what happens? Does the ping thread help reconnect? On my side it doesn't so I'm a bit surprised.

dktr0 commented 2 years ago

@domenkozar unfortunately I haven't had the chance to investigate further, so all I can really say about this is from the standpoint of observing what happens in a production deployment (of Estuary, a collaborative live coding platform https://github.com/dktr0/Estuary) when it is left running for a long time.

For example, currently the main deployment of this has been running continuously for 29 days and has handled just under 5000 sessions in that time. In the same time, the server seems to have accumulated about ~25 of these phantom connections. The application does have a ping thread, FWIW.

domenkozar commented 2 years ago

@dktr0 do you use a proper implementation of ping-pong? More in this thread: https://github.com/jaspervdj/websockets/issues/159

dktr0 commented 2 years ago

@domenkozar ah, I'm almost definitely not doing it right. Many thanks for this tip!