It's an issue in classic Smokey and NG, but classic Smokey had an easier time working around it because it didn't store the report data (just parsed it out of the message when it wanted it instead)
When you send a message via CE, it's done asynchronously in the sense that we don't get the message back (along with things like the message ID) until it comes down the websocket
When it comes down the websocket, the message that we posted goes into the callback for new messages like any other message
So what both versions of Smokey did was post the message and acquire a lock, then in the callback check to see if the new message was posted by SmokeDetector and if it is add it to our list of SmokeDetector messages and exit the critical region
Original recipe SmokeDetector didn't really think about the order the messages came back in (which was an oversight tbh), so it didn't acquire the lock until after the callback was called. In essence, the critical region was just the part where it was adding the message to the list
NG has one threading.Event
for every room, and waits on it and then clears it before posting a message. The callback then sets it when it receives a message, which wakes up any threads that wanted to post another message to this room and was waiting
By waiting until we got a message back before posting another one, this tried to ensure that we got the messages in the order that they were actually sent
Of course, something is going wrong, and the event is not getting set causing a deadlock. What is causing that isn't important, what's important is that we shouldn't need the lock: when we post the message to /chats/{room}/messages/new
, we get the ID of the message back immediately in the HTTP response. The only reason why we wait for it to come down the websocket is because CE discards the ID rather than returning it to us
@Andy does that flow kind of make sense?