From d3df69478dd965e7f26fa98bbf7bdd71316a43f0 Mon Sep 17 00:00:00 2001 From: Simo Piiroinen Date: Tue, 2 Apr 2019 09:26:17 +0300 Subject: [PATCH] [worker] Provide thread safe access to current mode data. JB#45312 The currently active mode data is owned by the worker thread. Accessing it from the main thread is not safe. Add worker_dup_usb_mode_data() function for getting copy of the currently active mode data in thread safe manner and utilize it from functions that might get called from the main thread too. Signed-off-by: Simo Piiroinen --- src/usb_moded-config.c | 64 +++++++++++++++++++---------------------- src/usb_moded-network.c | 4 +-- src/usb_moded-worker.c | 33 +++++++++++++++++++-- src/usb_moded-worker.h | 1 + 4 files changed, 64 insertions(+), 38 deletions(-) diff --git a/src/usb_moded-config.c b/src/usb_moded-config.c index 3b132d4..2ae9fa7 100644 --- a/src/usb_moded-config.c +++ b/src/usb_moded-config.c @@ -617,54 +617,50 @@ set_config_result_t config_set_network_setting(const char *config, const char *s return SET_CONFIG_ERROR; } -char * config_get_network_setting(const char *config) +char *config_get_network_setting(const char *config) { LOG_REGISTER_CONTEXT; - char * ret = 0; + char *ret = 0; - if(!strcmp(config, NETWORK_IP_KEY)) - { - ret = config_get_network_ip(); - if(!ret) - ret = strdup("192.168.2.15"); - } - else if(!strcmp(config, NETWORK_INTERFACE_KEY)) - { + modedata_t *data = 0; + if( !g_strcmp0(config, NETWORK_IP_KEY) ) { + if( !(ret = config_get_network_ip()) ) + ret = g_strdup("192.168.2.15"); + } + else if( !g_strcmp0(config, NETWORK_INTERFACE_KEY)) { /* check main configuration before using * the information from the specific mode */ - ret = config_get_network_interface(); + if( (ret = config_get_network_interface()) ) + goto EXIT; - if(ret) - goto end; /* no interface override specified, let's use the one * from the mode config */ - const modedata_t *data = worker_get_usb_mode_data(); - if(data) - { - if(data->network_interface) - { - ret = strdup(data->network_interface); - goto end; - } + if( (data = worker_dup_usb_mode_data()) ) { + if( (ret = g_strdup(data->network_interface)) ) + goto EXIT; } - ret = strdup("usb0"); + + ret = g_strdup("usb0"); } - else if(!strcmp(config, NETWORK_GATEWAY_KEY)) - return config_get_network_gateway(); - else if(!strcmp(config, NETWORK_NETMASK_KEY)) - { - ret = config_get_network_netmask(); - if(!ret) - ret = strdup("255.255.255.0"); + else if( !g_strcmp0(config, NETWORK_GATEWAY_KEY) ) { + ret = config_get_network_gateway(); + } + else if( !g_strcmp0(config, NETWORK_NETMASK_KEY) ) { + if( !(ret = config_get_network_netmask()) ) + ret = g_strdup("255.255.255.0"); } - else if(!strcmp(config, NETWORK_NAT_INTERFACE_KEY)) - return config_get_network_nat_interface(); - else + else if( !g_strcmp0(config, NETWORK_NAT_INTERFACE_KEY) ) { + ret = config_get_network_nat_interface(); + } + else { /* no matching keys, return error */ - return NULL; -end: + } + +EXIT: + modedata_free(data); + return ret; } diff --git a/src/usb_moded-network.c b/src/usb_moded-network.c index 2e8a588..b686f2a 100644 --- a/src/usb_moded-network.c +++ b/src/usb_moded-network.c @@ -1295,12 +1295,12 @@ int network_update(void) LOG_REGISTER_CONTEXT; if( control_get_cable_state() == CABLE_STATE_PC_CONNECTED ) { - // FIXME: data hazard - const modedata_t *data = worker_get_usb_mode_data(); + modedata_t *data = worker_dup_usb_mode_data(); if( data && data->network ) { network_down(data); network_up(data); } + modedata_free(data); } return 0; } diff --git a/src/usb_moded-worker.c b/src/usb_moded-worker.c index 16b8946..cc880da 100644 --- a/src/usb_moded-worker.c +++ b/src/usb_moded-worker.c @@ -61,6 +61,7 @@ const char *worker_get_kernel_module (void); bool worker_set_kernel_module (const char *module); void worker_clear_kernel_module (void); const modedata_t *worker_get_usb_mode_data (void); +modedata_t *worker_dup_usb_mode_data (void); void worker_set_usb_mode_data (const modedata_t *data); static const char *worker_get_activated_mode_locked(void); static bool worker_set_activated_mode_locked(const char *mode); @@ -395,8 +396,9 @@ static modedata_t *worker_mode_data = NULL; /** get the usb mode data * - * @return a pointer to the usb mode data + * Note: This function should be called only from the worker thread. * + * @return a pointer to the usb mode data */ const modedata_t *worker_get_usb_mode_data(void) { @@ -405,17 +407,41 @@ const modedata_t *worker_get_usb_mode_data(void) return worker_mode_data; } +/** get clone of the usb mode data + * + * Caller must release the returned object via #modedata_free(). + * + * @return a pointer to the usb mode data + */ +modedata_t *worker_dup_usb_mode_data(void) +{ + LOG_REGISTER_CONTEXT; + + WORKER_LOCKED_ENTER; + + modedata_t *modedata = modedata_copy(worker_mode_data); + + WORKER_LOCKED_LEAVE; + + return modedata;; +} + /** set the modedata_t data * - * @param data mode_list_element pointer + * Note: This function should be called only from the worker thread, * + * @param data mode_list_element pointer */ void worker_set_usb_mode_data(const modedata_t *data) { LOG_REGISTER_CONTEXT; + WORKER_LOCKED_ENTER; + modedata_free(worker_mode_data), worker_mode_data = modedata_copy(data); + + WORKER_LOCKED_LEAVE; } /* ------------------------------------------------------------------------- * @@ -962,6 +988,9 @@ worker_quit(void) worker_stop_thread(); worker_delete_eventfd(); + + /* Worker thread is stopped and resources can be released. */ + worker_set_usb_mode_data(0); } void diff --git a/src/usb_moded-worker.h b/src/usb_moded-worker.h index 99b5a2a..852e76f 100644 --- a/src/usb_moded-worker.h +++ b/src/usb_moded-worker.h @@ -45,6 +45,7 @@ const char *worker_get_kernel_module (void); bool worker_set_kernel_module (const char *module); void worker_clear_kernel_module (void); const modedata_t *worker_get_usb_mode_data (void); +modedata_t *worker_dup_usb_mode_data (void); void worker_set_usb_mode_data (const modedata_t *data); void worker_request_hardware_mode(const char *mode); void worker_clear_hardware_mode (void);