Add locking around sockets, since QUdpSocket isn't thread safe enough. #1122
Conversation
JulianGro
added
the
server
|
Server side, this appears to be working fine. At least 74 and I were able to connect to anime-avatars, which is running 964dbbb right now. |
| @@ -135,6 +138,8 @@ private slots: | |||
|
|
|||
| Mutex _unreliableSequenceNumbersMutex; | |||
| Mutex _connectionsHashMutex; | |||
| Mutex _socketMutex; | |||
There was a problem hiding this comment.
since the problem is QUdpSocket (and WebRTCSocket?), should this locking logic be internal to NetworkSocket.h instead?
There was a problem hiding this comment.
I think it makes more sense to keep it in udt::Socket.
NetworkSocket.h is an abstraction that could be used in a place where it's only ever used in a single thread and therefore not need locking. The problem happens because udt::Socket is called from multiple threads. And it also already deals with locking for other purposes.
There was a problem hiding this comment.
maybe, but it could just as easily be used in another multithreaded context and then we open ourselves up to more difficult to debug threading crashes 😄
at the very least, can we add something to NetworkSocket about this? maybe just a big comment, although if we could detect this and log a warning (just the first time it happens) that would also be great.
This fixes #1119
After this it's clear the networking code still needs work as in suboptimal conditions it may fail to connect to a domain or keep dropping off. But at least it doesn't crash.