Skip to content

Commit b7f10ca

Browse files
c-jimenezc-jimenez
authored
Merge pull request #210 from c-jimenez/fix/segfault_on_client_pool
[websockets] Handle clean disconnection when disconnect request occur…
2 parents bdb36b4 + 8889bb3 commit b7f10ca

File tree

2 files changed

+138
-114
lines changed

2 files changed

+138
-114
lines changed

‎src/websockets/libwebsockets/LibWebsocketClientPool.cpp‎

Lines changed: 136 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -211,10 +211,17 @@ int LibWebsocketClientPool::eventCallback(struct lws* wsi, enum lws_callback_rea
211211
{
212212
lws_set_timeout(waiting_client->m_wsi, static_cast<pending_timeout>(1), LWS_TO_KILL_SYNC);
213213
}
214-
lws_vhost_destroy(waiting_client->m_vhost);
215-
waiting_client->m_vhost = nullptr;
216-
waiting_client->m_connected = false;
217-
waiting_client->m_disconnect_process_done = true;
214+
if (waiting_client->m_vhost)
215+
{
216+
lws_vhost_destroy(waiting_client->m_vhost);
217+
}
218+
219+
std::lock_guard<std::mutex> lock(waiting_client->m_disconnect_mutex);
220+
waiting_client->m_protocol.clear();
221+
waiting_client->m_vhost = nullptr;
222+
waiting_client->m_connected = false;
223+
waiting_client->m_disconnect_process_in_progress = false;
224+
waiting_client->m_disconnect_process_done = true;
218225
waiting_client->m_disconnect_cond_var.notify_all();
219226
}
220227
}
@@ -242,6 +249,7 @@ LibWebsocketClientPool::Client::Client(LibWebsocketClientPool& pool)
242249
m_connected(false),
243250
m_disconnect_cond_var(),
244251
m_disconnect_mutex(),
252+
m_disconnect_process_in_progress(false),
245253
m_disconnect_process_done(false),
246254
m_context(m_pool.m_context),
247255
m_vhost(nullptr),
@@ -282,6 +290,8 @@ bool LibWebsocketClientPool::Client::connect(const std::string& url,
282290
{
283291
bool ret = false;
284292

293+
std::lock_guard<std::mutex> lock(m_disconnect_mutex);
294+
285295
// Check if thread is alive and if a listener has been registered
286296
if (!m_vhost && m_listener)
287297
{
@@ -334,19 +344,21 @@ bool LibWebsocketClientPool::Client::disconnect()
334344
{
335345
bool ret = false;
336346

347+
std::unique_lock<std::mutex> lock(m_disconnect_mutex);
348+
337349
// Check if connected
338-
if (m_vhost)
350+
if (!m_disconnect_process_in_progress && !m_protocol.empty())
339351
{
340352
// Schedule disconnection
341-
m_retry_interval = 0;
342-
m_disconnect_process_done = false;
353+
m_retry_interval = 0;
354+
m_disconnect_process_in_progress = true;
355+
m_disconnect_process_done = false;
343356
m_pool.m_waiting_disconnect_queue.push(this);
344357
lws_cancel_service(m_context);
345358

346359
// Wait actual disconnection
347360
if (std::this_thread::get_id() != m_pool.m_thread->get_id())
348361
{
349-
std::unique_lock<std::mutex> lock(m_disconnect_mutex);
350362
m_disconnect_cond_var.wait(lock, [&] { return m_disconnect_process_done; });
351363
}
352364
}
@@ -374,6 +386,8 @@ bool LibWebsocketClientPool::Client::send(const void* data, size_t size)
374386
{
375387
bool ret = false;
376388

389+
std::lock_guard<std::mutex> lock(m_disconnect_mutex);
390+
377391
// Check if connected
378392
if (m_connected)
379393
{
@@ -434,136 +448,144 @@ void LibWebsocketClientPool::Client::connectCallback(struct lws_sorted_usec_list
434448
ScheduleData* schedule_data = lws_container_of(sul, ScheduleData, sched_list);
435449
if (schedule_data)
436450
{
437-
// Check if vhost has been created
438-
Client* client = schedule_data->client;
439-
if (!client->m_vhost)
451+
Client* client = schedule_data->client;
452+
std::lock_guard<std::mutex> lock(client->m_disconnect_mutex);
453+
454+
// Check if a disconnect process is in progress
455+
if (!client->m_disconnect_process_in_progress)
440456
{
441-
// Define callback
442-
struct lws_protocols protocols[] = {
443-
{"LibWebsocketClientPoolClient", &LibWebsocketClientPool::Client::eventCallback, 0, 0, 0, client, 0},
444-
LWS_PROTOCOL_LIST_TERM};
445-
446-
// Fill vhost information
447-
struct lws_context_creation_info vhost_info;
448-
memset(&vhost_info, 0, sizeof(vhost_info));
449-
vhost_info.options = LWS_SERVER_OPTION_DO_SSL_GLOBAL_INIT;
450-
vhost_info.port = CONTEXT_PORT_NO_LISTEN;
451-
vhost_info.timeout_secs = client->m_connect_timeout;
452-
vhost_info.connect_timeout_secs = client->m_connect_timeout;
453-
vhost_info.protocols = protocols;
454-
vhost_info.log_cx = &pool->m_logs_context;
455-
if (client->m_url.protocol() == "wss")
457+
// Check if vhost has been created
458+
if (!client->m_vhost)
456459
{
457-
if (!client->m_credentials.tls12_cipher_list.empty())
460+
// Define callback
461+
struct lws_protocols protocols[] = {
462+
{"LibWebsocketClientPoolClient", &LibWebsocketClientPool::Client::eventCallback, 0, 0, 0, client, 0},
463+
LWS_PROTOCOL_LIST_TERM};
464+
465+
// Fill vhost information
466+
struct lws_context_creation_info vhost_info;
467+
memset(&vhost_info, 0, sizeof(vhost_info));
468+
vhost_info.options = LWS_SERVER_OPTION_DO_SSL_GLOBAL_INIT;
469+
vhost_info.port = CONTEXT_PORT_NO_LISTEN;
470+
vhost_info.timeout_secs = client->m_connect_timeout;
471+
vhost_info.connect_timeout_secs = client->m_connect_timeout;
472+
vhost_info.protocols = protocols;
473+
vhost_info.log_cx = &pool->m_logs_context;
474+
if (client->m_url.protocol() == "wss")
458475
{
459-
vhost_info.client_ssl_cipher_list = client->m_credentials.tls12_cipher_list.c_str();
460-
}
461-
if (!client->m_credentials.tls13_cipher_list.empty())
462-
{
463-
vhost_info.client_tls_1_3_plus_cipher_list = client->m_credentials.tls13_cipher_list.c_str();
464-
}
465-
if (client->m_credentials.encoded_pem_certificates)
466-
{
467-
// Use PEM encoded data
468-
if (!client->m_credentials.server_certificate_ca.empty())
476+
if (!client->m_credentials.tls12_cipher_list.empty())
469477
{
470-
vhost_info.client_ssl_ca_mem = client->m_credentials.server_certificate_ca.c_str();
471-
vhost_info.client_ssl_ca_mem_len = static_cast<unsigned int>(client->m_credentials.server_certificate_ca.size());
478+
vhost_info.client_ssl_cipher_list = client->m_credentials.tls12_cipher_list.c_str();
472479
}
473-
if (!client->m_credentials.client_certificate.empty())
480+
if (!client->m_credentials.tls13_cipher_list.empty())
474481
{
475-
vhost_info.client_ssl_cert_mem = client->m_credentials.client_certificate.c_str();
476-
vhost_info.client_ssl_cert_mem_len = static_cast<unsigned int>(client->m_credentials.client_certificate.size());
482+
vhost_info.client_tls_1_3_plus_cipher_list = client->m_credentials.tls13_cipher_list.c_str();
477483
}
478-
if (!client->m_credentials.client_certificate_private_key.empty())
484+
if (client->m_credentials.encoded_pem_certificates)
479485
{
480-
vhost_info.client_ssl_key_mem = client->m_credentials.client_certificate_private_key.c_str();
481-
vhost_info.client_ssl_key_mem_len =
482-
static_cast<unsigned int>(client->m_credentials.client_certificate_private_key.size());
486+
// Use PEM encoded data
487+
if (!client->m_credentials.server_certificate_ca.empty())
488+
{
489+
vhost_info.client_ssl_ca_mem = client->m_credentials.server_certificate_ca.c_str();
490+
vhost_info.client_ssl_ca_mem_len =
491+
static_cast<unsigned int>(client->m_credentials.server_certificate_ca.size());
492+
}
493+
if (!client->m_credentials.client_certificate.empty())
494+
{
495+
vhost_info.client_ssl_cert_mem = client->m_credentials.client_certificate.c_str();
496+
vhost_info.client_ssl_cert_mem_len = static_cast<unsigned int>(client->m_credentials.client_certificate.size());
497+
}
498+
if (!client->m_credentials.client_certificate_private_key.empty())
499+
{
500+
vhost_info.client_ssl_key_mem = client->m_credentials.client_certificate_private_key.c_str();
501+
vhost_info.client_ssl_key_mem_len =
502+
static_cast<unsigned int>(client->m_credentials.client_certificate_private_key.size());
503+
}
483504
}
484-
}
485-
else
486-
{
487-
// Load PEM files from filesystem
488-
if (!client->m_credentials.server_certificate_ca.empty())
505+
else
489506
{
490-
vhost_info.client_ssl_ca_filepath = client->m_credentials.server_certificate_ca.c_str();
507+
// Load PEM files from filesystem
508+
if (!client->m_credentials.server_certificate_ca.empty())
509+
{
510+
vhost_info.client_ssl_ca_filepath = client->m_credentials.server_certificate_ca.c_str();
511+
}
512+
if (!client->m_credentials.client_certificate.empty())
513+
{
514+
vhost_info.client_ssl_cert_filepath = client->m_credentials.client_certificate.c_str();
515+
}
516+
if (!client->m_credentials.client_certificate_private_key.empty())
517+
{
518+
vhost_info.client_ssl_private_key_filepath = client->m_credentials.client_certificate_private_key.c_str();
519+
}
491520
}
492-
if (!client->m_credentials.client_certificate.empty())
521+
if (!client->m_credentials.client_certificate_private_key_passphrase.empty())
493522
{
494-
vhost_info.client_ssl_cert_filepath = client->m_credentials.client_certificate.c_str();
523+
vhost_info.client_ssl_private_key_password =
524+
client->m_credentials.client_certificate_private_key_passphrase.c_str();
495525
}
496-
if (!client->m_credentials.client_certificate_private_key.empty())
497-
{
498-
vhost_info.client_ssl_private_key_filepath = client->m_credentials.client_certificate_private_key.c_str();
499-
}
500-
}
501-
if (!client->m_credentials.client_certificate_private_key_passphrase.empty())
502-
{
503-
vhost_info.client_ssl_private_key_password = client->m_credentials.client_certificate_private_key_passphrase.c_str();
504526
}
505-
}
506527

507-
// Create vhost
508-
client->m_vhost = lws_create_vhost(client->m_context, &vhost_info);
509-
}
510-
if (client->m_vhost)
511-
{
512-
// Connexion parameters
513-
struct lws_client_connect_info connect_info;
514-
memset(&connect_info, 0, sizeof(connect_info));
515-
connect_info.context = client->m_context;
516-
connect_info.vhost = client->m_vhost;
517-
connect_info.address = client->m_url.address().c_str();
518-
connect_info.path = client->m_url.path().c_str();
519-
connect_info.host = connect_info.address;
520-
connect_info.origin = connect_info.address;
521-
if (client->m_url.protocol() == "wss")
528+
// Create vhost
529+
client->m_vhost = lws_create_vhost(client->m_context, &vhost_info);
530+
}
531+
if (client->m_vhost)
522532
{
523-
connect_info.ssl_connection = LCCSCF_USE_SSL;
524-
if (client->m_credentials.allow_selfsigned_certificates)
533+
// Connexion parameters
534+
struct lws_client_connect_info connect_info;
535+
memset(&connect_info, 0, sizeof(connect_info));
536+
connect_info.context = client->m_context;
537+
connect_info.vhost = client->m_vhost;
538+
connect_info.address = client->m_url.address().c_str();
539+
connect_info.path = client->m_url.path().c_str();
540+
connect_info.host = connect_info.address;
541+
connect_info.origin = connect_info.address;
542+
if (client->m_url.protocol() == "wss")
525543
{
526-
connect_info.ssl_connection |= LCCSCF_ALLOW_SELFSIGNED;
544+
connect_info.ssl_connection = LCCSCF_USE_SSL;
545+
if (client->m_credentials.allow_selfsigned_certificates)
546+
{
547+
connect_info.ssl_connection |= LCCSCF_ALLOW_SELFSIGNED;
548+
}
549+
if (client->m_credentials.allow_expired_certificates)
550+
{
551+
connect_info.ssl_connection |= LCCSCF_ALLOW_EXPIRED;
552+
}
553+
if (client->m_credentials.accept_untrusted_certificates)
554+
{
555+
connect_info.ssl_connection |= LCCSCF_ALLOW_INSECURE;
556+
}
557+
if (client->m_credentials.skip_server_name_check)
558+
{
559+
connect_info.ssl_connection |= LCCSCF_SKIP_SERVER_CERT_HOSTNAME_CHECK;
560+
}
561+
connect_info.port = 443;
527562
}
528-
if (client->m_credentials.allow_expired_certificates)
563+
else
529564
{
530-
connect_info.ssl_connection |= LCCSCF_ALLOW_EXPIRED;
565+
connect_info.port = 80;
531566
}
532-
if (client->m_credentials.accept_untrusted_certificates)
567+
if (client->m_url.port())
533568
{
534-
connect_info.ssl_connection |= LCCSCF_ALLOW_INSECURE;
569+
connect_info.port = static_cast<int>(client->m_url.port());
535570
}
536-
if (client->m_credentials.skip_server_name_check)
571+
connect_info.protocol = client->m_protocol.c_str();
572+
connect_info.local_protocol_name = "LibWebsocketClientPoolClient";
573+
connect_info.pwsi = &client->m_wsi;
574+
connect_info.retry_and_idle_policy = &client->m_retry_policy;
575+
connect_info.userdata = client;
576+
577+
// Start connection
578+
if (!lws_client_connect_via_info(&connect_info))
537579
{
538-
connect_info.ssl_connection |= LCCSCF_SKIP_SERVER_CERT_HOSTNAME_CHECK;
580+
// Schedule a retry
581+
client->m_retry_count = 0;
582+
lws_retry_sul_schedule(pool->m_context,
583+
0,
584+
sul,
585+
&client->m_retry_policy,
586+
&LibWebsocketClientPool::Client::connectCallback,
587+
&client->m_retry_count);
539588
}
540-
connect_info.port = 443;
541-
}
542-
else
543-
{
544-
connect_info.port = 80;
545-
}
546-
if (client->m_url.port())
547-
{
548-
connect_info.port = static_cast<int>(client->m_url.port());
549-
}
550-
connect_info.protocol = client->m_protocol.c_str();
551-
connect_info.local_protocol_name = "LibWebsocketClientPoolClient";
552-
connect_info.pwsi = &client->m_wsi;
553-
connect_info.retry_and_idle_policy = &client->m_retry_policy;
554-
connect_info.userdata = client;
555-
556-
// Start connection
557-
if (!lws_client_connect_via_info(&connect_info))
558-
{
559-
// Schedule a retry
560-
client->m_retry_count = 0;
561-
lws_retry_sul_schedule(pool->m_context,
562-
0,
563-
sul,
564-
&client->m_retry_policy,
565-
&LibWebsocketClientPool::Client::connectCallback,
566-
&client->m_retry_count);
567589
}
568590
}
569591
}

‎src/websockets/libwebsockets/LibWebsocketClientPool.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,8 @@ class LibWebsocketClientPool
171171
std::condition_variable m_disconnect_cond_var;
172172
/** @brief Disconnect mutex */
173173
std::mutex m_disconnect_mutex;
174+
/** @brief Indicate that the disconnect process is in progress */
175+
bool m_disconnect_process_in_progress;
174176
/** @brief Indicate that the disconnect process is done */
175177
bool m_disconnect_process_done;
176178

0 commit comments

Comments
 (0)