[Libvirt-cim] [PATCH V5 06/15] CSI, add lock to protect shared data in lifecycle_thread

[Wenchao Xia]

  This patch use lock 'lifecycle_mutex' to protect gloable shared
data in lifecycle_thread(). This lock exist in Activate/Deactivate
Enable/Disable method, which is meant to protect the gloable
shared data, but forgot to be added in new libvirt based CSI
thread.
  This patch can avoid following risk at least: Original code have
a small chance to free thread->args in child thread just after main
thread malloc it, for that thread->id is set to zero allowing main
thread to enter that code.
  This patch focus on adding missing lock, the CSI can be still
improved as: smaller lock, folder lock into a structure with data
to tip better what it is doing.

Signed-off-by: Wenchao Xia <xiawenc at linux.vnet.ibm.com>
---
 src/Virt_ComputerSystemIndication.c |   39 ++++++++++++++++++++++++++++++++--
 1 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c
index 8250850..3df47fa 100644
--- a/src/Virt_ComputerSystemIndication.c
+++ b/src/Virt_ComputerSystemIndication.c
@@ -174,9 +174,11 @@ static void csi_free_thread_data(void *data)
         if (data == NULL)
                 return;
 
+        pthread_mutex_lock(&lifecycle_mutex);
         list_free(thread->dom_list);
         thread->dom_list = NULL;
         stdi_free_ind_args(&thread->args);
+        pthread_mutex_unlock(&lifecycle_mutex);
 }
 
 void set_source_inst_props(const CMPIBroker *broker,
@@ -491,6 +493,8 @@ static int update_domain_list(virConnectPtr conn, csi_thread_data_t *thread)
         return s.rc;
 }
 
+/* following function will protect global data with lifecycle_mutex.
+   TODO: improve it with seperate lock later. */
 static void csi_domain_event_cb(virConnectPtr conn,
                                 virDomainPtr dom,
                                 int event,
@@ -503,10 +507,13 @@ static void csi_domain_event_cb(virConnectPtr conn,
         char *prefix = class_prefix_name(thread->args->classname);
         CMPIStatus s = {CMPI_RC_OK, NULL};
 
+        pthread_mutex_lock(&lifecycle_mutex);
         if (lifecycle_enabled == false || thread->active_filters <= 0) {
                 CU_DEBUG("%s indications deactivated, return", prefix);
+                pthread_mutex_unlock(&lifecycle_mutex);
                 return;
         }
+        pthread_mutex_unlock(&lifecycle_mutex);
 
         CU_DEBUG("Event: Domain %s(%d) event: %d detail: %d\n",
                  virDomainGetName(dom), virDomainGetID(dom), event, detail);
@@ -538,7 +545,9 @@ static void csi_domain_event_cb(virConnectPtr conn,
         if (cs_event != CS_CREATED) {
                 char uuid[VIR_UUID_STRING_BUFLEN] = {0};
                 virDomainGetUUIDString(dom, &uuid[0]);
+                pthread_mutex_lock(&lifecycle_mutex);
                 dom_xml = list_find(thread->dom_list, uuid);
+                pthread_mutex_unlock(&lifecycle_mutex);
         }
 
         if (dom_xml == NULL) {
@@ -546,12 +555,16 @@ static void csi_domain_event_cb(virConnectPtr conn,
                 goto end;
         }
 
+        pthread_mutex_lock(&lifecycle_mutex);
         async_ind(thread->args, cs_event, dom_xml, prefix);
+        pthread_mutex_unlock(&lifecycle_mutex);
 
         /* Update the domain list accordingly */
         if (event == VIR_DOMAIN_EVENT_DEFINED) {
                 if (detail == VIR_DOMAIN_EVENT_DEFINED_ADDED) {
+                        pthread_mutex_lock(&lifecycle_mutex);
                         csi_thread_dom_list_append(thread, dom_xml);
+                        pthread_mutex_unlock(&lifecycle_mutex);
                 } else if (detail == VIR_DOMAIN_EVENT_DEFINED_UPDATED) {
                         free(dom_xml->name);
                         free(dom_xml->xml);
@@ -559,7 +572,9 @@ static void csi_domain_event_cb(virConnectPtr conn,
                 }
         } else if (event == VIR_DOMAIN_EVENT_DEFINED &&
                    detail == VIR_DOMAIN_EVENT_UNDEFINED_REMOVED) {
+                pthread_mutex_lock(&lifecycle_mutex);
                 list_remove(thread->dom_list, dom_xml);
+                pthread_mutex_unlock(&lifecycle_mutex);
         }
 
  end:
@@ -580,10 +595,12 @@ static CMPI_THREAD_RETURN lifecycle_thread(void *params)
         if (prefix == NULL)
                 goto init_out;
 
+        pthread_mutex_lock(&lifecycle_mutex);
         conn = connect_by_classname(_BROKER, args->classname, &s);
         if (conn == NULL) {
                 CU_DEBUG("Unable to start lifecycle thread: "
                          "Failed to connect (cn: %s)", args->classname);
+                pthread_mutex_unlock(&lifecycle_mutex);
                 goto conn_out;
         }
 
@@ -595,33 +612,47 @@ static CMPI_THREAD_RETURN lifecycle_thread(void *params)
 
         if (cb_id == -1) {
                 CU_DEBUG("Failed to register domain event watch for '%s'",
-                         args->classname)
+                         args->classname);
+                pthread_mutex_unlock(&lifecycle_mutex);
                 goto cb_out;
         }
 
         CBAttachThread(_BROKER, args->context);
 
         /* Get currently defined domains */
-        if (update_domain_list(conn, thread) != CMPI_RC_OK)
+        if (update_domain_list(conn, thread) != CMPI_RC_OK) {
+                pthread_mutex_unlock(&lifecycle_mutex);
                 goto end;
+        }
 
+        pthread_mutex_unlock(&lifecycle_mutex);
 
         CU_DEBUG("Entering CSI event loop (%s)", prefix);
-        while (thread->active_filters > 0) {
+        while (1) {
+                pthread_mutex_lock(&lifecycle_mutex);
+                if (thread->active_filters <= 0) {
+                        pthread_mutex_unlock(&lifecycle_mutex);
+                        break;
+                }
+                pthread_mutex_unlock(&lifecycle_mutex);
                 if (virEventRunDefaultImpl() < 0) {
                         virErrorPtr err = virGetLastError();
                         CU_DEBUG("Failed to run event loop: %s\n",
                         err && err->message ? err->message : "Unknown error");
                 }
+                usleep(1);
         }
 
         CU_DEBUG("Exiting CSI event loop (%s)", prefix);
 
+        pthread_mutex_lock(&lifecycle_mutex);
         CBDetachThread(_BROKER, args->context);
+        pthread_mutex_unlock(&lifecycle_mutex);
  end:
         virConnectDomainEventDeregisterAny(conn, cb_id);
 
  cb_out:
+        pthread_mutex_lock(&lifecycle_mutex);
 
         thread->id = 0;
         thread->active_filters = 0;
@@ -629,6 +660,8 @@ static CMPI_THREAD_RETURN lifecycle_thread(void *params)
         if (thread->args != NULL)
                 stdi_free_ind_args(&thread->args);
 
+        pthread_mutex_unlock(&lifecycle_mutex);
+
  conn_out:
         virConnectClose(conn);
 
-- 
1.7.1