Skip to content

Commit 7224d35

Browse files
authored
Merge pull request microsoft#217 from daniuk-microsoft/daniuk/ws_client_winrt/race_condition_fix
Fixed race condition in Casablanca ws_client_winrt.cpp
2 parents 4d52eae + c800ab2 commit 7224d35

File tree

1 file changed

+29
-17
lines changed

1 file changed

+29
-17
lines changed

Release/src/websockets/client/ws_client_winrt.cpp

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,7 @@ class winrt_callback_client : public websocket_client_callback_impl, public std:
8585
public:
8686
winrt_callback_client(websocket_client_config config) :
8787
websocket_client_callback_impl(std::move(config)),
88-
m_connected(false),
89-
m_num_sends(0)
88+
m_connected(false)
9089
{
9190
m_msg_websocket = ref new MessageWebSocket();
9291

@@ -243,17 +242,24 @@ class winrt_callback_client : public websocket_client_callback_impl, public std:
243242
return pplx::task_from_exception<void>(websocket_exception("Message size too large. Ensure message length is less than UINT_MAX."));
244243
}
245244

246-
if (++m_num_sends == 1) // No sends in progress
245+
bool msg_pending = false;
247246
{
248-
// Start sending the message
249-
send_msg(msg);
250-
}
251-
else
252-
{
253-
// Only actually have to take the lock if touching the queue.
254247
std::lock_guard<std::mutex> lock(m_send_lock);
248+
if (m_outgoing_msg_queue.size() > 0)
249+
{
250+
msg_pending = true;
251+
}
252+
255253
m_outgoing_msg_queue.push(msg);
256254
}
255+
256+
// No sends in progress
257+
if (msg_pending == false)
258+
{
259+
// Start sending the message
260+
send_msg(msg);
261+
}
262+
257263
return pplx::create_task(msg.body_sent());
258264
}
259265

@@ -385,15 +391,24 @@ class winrt_callback_client : public websocket_client_callback_impl, public std:
385391
msg.signal_body_sent();
386392
}
387393

388-
if (--this_client->m_num_sends > 0)
394+
bool msg_pending = false;
395+
websocket_outgoing_message next_msg;
389396
{
390397
// Only hold the lock when actually touching the queue.
391-
websocket_outgoing_message next_msg;
398+
std::lock_guard<std::mutex> lock(this_client->m_send_lock);
399+
400+
// First message in queue has been sent
401+
this_client->m_outgoing_msg_queue.pop();
402+
403+
if (this_client->m_outgoing_msg_queue.size() > 0)
392404
{
393-
std::lock_guard<std::mutex> lock(this_client->m_send_lock);
394405
next_msg = this_client->m_outgoing_msg_queue.front();
395-
this_client->m_outgoing_msg_queue.pop();
406+
msg_pending = true;
396407
}
408+
}
409+
410+
if (msg_pending)
411+
{
397412
this_client->send_msg(next_msg);
398413
}
399414
});
@@ -443,11 +458,8 @@ class winrt_callback_client : public websocket_client_callback_impl, public std:
443458
// The implementation has to ensure ordering of send requests
444459
std::mutex m_send_lock;
445460

446-
// Queue to order the sends
461+
// Queue to track pending sends
447462
std::queue<websocket_outgoing_message> m_outgoing_msg_queue;
448-
449-
// Number of sends in progress and queued up.
450-
std::atomic<int> m_num_sends;
451463
};
452464

453465
void ReceiveContext::OnReceive(MessageWebSocket^ sender, MessageWebSocketMessageReceivedEventArgs^ args)

0 commit comments

Comments
 (0)