osaf/libs/agents/saf/lga/lga_state.c | 41 +++++++++++++++++++++++++++++++---- osaf/libs/agents/saf/lga/lga_state.h | 1 + osaf/libs/agents/saf/lga/lga_util.c | 30 +++++++++++-------------- 3 files changed, 50 insertions(+), 22 deletions(-)
There was an race condition between client thread and recovery thread accessing to common resource `client_list`. Add mutex protection. diff --git a/osaf/libs/agents/saf/lga/lga_state.c b/osaf/libs/agents/saf/lga/lga_state.c --- a/osaf/libs/agents/saf/lga/lga_state.c +++ b/osaf/libs/agents/saf/lga/lga_state.c @@ -355,8 +355,11 @@ static void *recovery2_thread(void *dumm /* We have been signaled to exit */ goto done; } + /* Recover clients one at a time */ rc = recover_one_client(p_client); + if (p_client == NULL) goto done; + TRACE("\t Client %d is recovered", p_client->lgs_client_id); if (rc == -1) { TRACE("%s recover_one_client Fail Deleting cllient (id %d)", @@ -593,6 +596,15 @@ int recover_one_client(lga_client_hdl_re TRACE_ENTER(); + if (p_client == NULL) { + /** + * Probably the client_list has been deleted by client thread + * in context calling saLogFinalize(). So, give up recovering. + */ + rc = -1; + goto done; + } + /* This function may be called both in the recovery thread and in the * client thread. A possible scenario is that the recovery 2 thread * times out and calls this function in recovery mode 2 while there @@ -658,13 +670,32 @@ uint32_t delete_one_client( lga_client_hdl_rec_t *rm_node ) { - TRACE_ENTER2(); - uint32_t ncs_rc; + return lga_hdl_rec_del(list_head, rm_node); +} +/** + * Free the memory allocated for one client handle with mutext protection. + * + * @param p_client_hdl + * @return void + */ +void free_client_hdl(lga_client_hdl_rec_t **p_client_hdl) +{ + lga_client_hdl_rec_t *client_hdl; + + /** + * To prevent race condition b/w client thread and recovery thread + * accessing to common resource `client_list`. [#1705] + */ osaf_mutex_lock_ordie(&lga_recov_mutex); - ncs_rc = lga_hdl_rec_del(list_head, rm_node); + + client_hdl = *p_client_hdl; + if (client_hdl == NULL) goto done; + + free(client_hdl); + client_hdl = NULL; + +done: osaf_mutex_unlock_ordie(&lga_recov_mutex); - TRACE_LEAVE(); - return ncs_rc; } diff --git a/osaf/libs/agents/saf/lga/lga_state.h b/osaf/libs/agents/saf/lga/lga_state.h --- a/osaf/libs/agents/saf/lga/lga_state.h +++ b/osaf/libs/agents/saf/lga/lga_state.h @@ -32,6 +32,7 @@ uint32_t delete_one_client( lga_client_hdl_rec_t **list_head, lga_client_hdl_rec_t *rm_node ); +void free_client_hdl(lga_client_hdl_rec_t **p_client_hdl); #ifdef __cplusplus } diff --git a/osaf/libs/agents/saf/lga/lga_util.c b/osaf/libs/agents/saf/lga/lga_util.c --- a/osaf/libs/agents/saf/lga/lga_util.c +++ b/osaf/libs/agents/saf/lga/lga_util.c @@ -19,6 +19,7 @@ #include <syslog.h> #include "lga.h" #include "osaf_poll.h" +#include "lga_state.h" /* Variables used during startup/shutdown only */ static pthread_mutex_t lga_lock = PTHREAD_MUTEX_INITIALIZER; @@ -449,11 +450,9 @@ void lga_hdl_list_del(lga_client_hdl_rec /** clean up the channel records for this lga-client **/ lga_log_stream_hdl_rec_list_del(&client_hdl->stream_list); - /** remove the association with hdl-mngr - **/ - free(client_hdl); - client_hdl = 0; + free_client_hdl(&client_hdl); } + TRACE_LEAVE(); } @@ -536,38 +535,35 @@ uint32_t lga_hdl_rec_del(lga_client_hdl_ if (list_iter == rm_node) { *list_head = rm_node->next; - /** detach & release the IPC - **/ + /* detach & release the IPC */ m_NCS_IPC_DETACH(&rm_node->mbx, lga_clear_mbx, NULL); m_NCS_IPC_RELEASE(&rm_node->mbx, NULL); ncshm_give_hdl(rm_node->local_hdl); ncshm_destroy_hdl(NCS_SERVICE_ID_LGA, rm_node->local_hdl); - /** Free the channel records off this hdl - **/ + + /* Free the channel records off this hdl */ lga_log_stream_hdl_rec_list_del(&rm_node->stream_list); + free_client_hdl(&rm_node); - /** free the hdl rec - **/ - free(rm_node); rc = NCSCC_RC_SUCCESS; goto out; - } else { /* find the rec */ + } else { + /* find the rec */ while (NULL != list_iter) { if (list_iter->next == rm_node) { list_iter->next = rm_node->next; - /** detach & release the IPC */ + /* detach & release the IPC */ m_NCS_IPC_DETACH(&rm_node->mbx, lga_clear_mbx, NULL); m_NCS_IPC_RELEASE(&rm_node->mbx, NULL); ncshm_give_hdl(rm_node->local_hdl); ncshm_destroy_hdl(NCS_SERVICE_ID_LGA, rm_node->local_hdl); - /** Free the channel records off this lga_hdl */ + + /* Free the channel records off this lga_hdl */ lga_log_stream_hdl_rec_list_del(&rm_node->stream_list); - - /** free the hdl rec */ - free(rm_node); + free_client_hdl(&rm_node); rc = NCSCC_RC_SUCCESS; goto out; ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140 _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel