indmanager: Rework thread cancellation again

The approach introduced in commit 381f4038a6a was on a good way however
turned out the pthread_cancel() actually queues a cancellation request
since we're using deferred cancellation mode and continues the code flow
until a function that is a cancellation point is called. We unlocked
the mutex before joining the thread which led to mutex acquisition by
the thread and then cancelling on a syscall, leaving the mutex locked
forever.

Having cancellation and join with mutex held was not a solution as it
would make the thread waiting for mutex leading to deadlock.

Instead, this patch introduces a private flag that will indicate the
thread should cancel itself. Two scenarios are possible:

 - the thread is doing unlocked stuff, typically waiting for events
   using syscalls that are cancellation points. In this case the
   pthread_cancel() will take effect immediately, breaking the syscall
   and quitting the thread. We're still in unlocked state.

 - the thread is holding the lock, in that case the im_stop_ind() will
   wait until that work is finished. It's better to leave the manage
   thread finish its job, it's mostly CMPI stuff and breaking in the
   middle would leak some memory. Once the main thread acquires the
   mutex, it cancels the thread, sets a private flag and unlocks again.
   While waiting for thread join is finished, the thread picks up the
   lock again as it was waiting for it and as a first thing it will
   check the private flag and quits gracefully, unlocking the mutex.
   As a side effect, the pthread cancellation machinery has no chance
   to kick in as there was no cancellation point on the way.

2 files changed, 26 insertions, 3 deletions

diff --git a/src/indmanager/ind_manager.c b/src/indmanager/ind_manager.c
index 462227e..d6cd650 100644
--- a/src/indmanager/ind_manager.c
+++ b/src/indmanager/ind_manager.c
@@ -677,6 +677,11 @@ static void *manage(void *data)
         DEBUG("manage thread in infinite loop");
         // wait until manager is running
         pthread_mutex_lock(&manager->_t_mutex);
+        if (manager->cancelled) {
+            pthread_mutex_unlock(&manager->_t_mutex);
+            err = IM_ERR_CANCELLED;
+            return (void *)err;
+        }
         while (!manager->running) {
             DEBUG("manage thread waiting to indication start");
             pthread_cond_wait(&manager->_t_cond, &manager->_t_mutex);
@@ -695,6 +700,11 @@ static void *manage(void *data)
             continue;
         }
         pthread_mutex_lock(&manager->_t_mutex);
+        if (manager->cancelled) {
+            pthread_mutex_unlock(&manager->_t_mutex);
+            err = IM_ERR_CANCELLED;
+            return (void *)err;
+        }
         if (manager->polling) {
             // poll enumerations
             if (!_im_poll(manager, &err)) {
@@ -819,6 +829,7 @@ IMManager* im_create_manager(IMInstGather gather, IMFilterChecker f_checker,
     manager->filters = NULL;
     manager->running = false;
     manager->polling = polling;
+    manager->cancelled = false;
     manager->broker = broker;
     manager->f_checker = f_checker;
     manager->enums = NULL;
@@ -1175,6 +1186,7 @@ bool im_start_ind(IMManager *manager, const CMPIContext *ctx, IMError *err)
         *err = IM_ERR_THREAD;
         return false;
     }
+    manager->cancelled = false;
     manager->ctx_main = ctx;
     manager->ctx_manage = CBPrepareAttachThread(manager->broker,
                                                 manager->ctx_main);
@@ -1205,17 +1217,25 @@ bool im_stop_ind(IMManager *manager, const CMPIContext *ctx, IMError *err)
     DEBUG("Stopping indications");
     manager->running = false;
 
-    /* We must lock the mutex so we are sure the thread does not hold it.
-     * If we destroy the thread with mutex locked, the mutex is locked forever.
-     */
+    /* First lock the mutex so we are sure the thread does not hold it. */
     pthread_mutex_lock(&manager->_t_mutex);
+    /* Then cancel the thread in deferred mode and set a private flag.
+     * The thread may be doing unlocked stuff that will cancel just fine
+     * or may be waiting for mutex acquisition where it will cancel and
+     * unlock right after that using our private flag.
+     */
+    manager->cancelled = true;
+    /* Note that mutex functions ARE NOT cancellation points! */
     if (pthread_cancel(manager->_t_manage)) {
         *err = IM_ERR_THREAD;
         pthread_mutex_unlock(&manager->_t_mutex);
         return false;
     }
+
+    /* Unlock the mutex and give the thread chance to cancel properly. */
     pthread_mutex_unlock(&manager->_t_mutex);
 
+    /* Wait until thread cancellation is finished. */
     if (pthread_join(manager->_t_manage, NULL)) {
         *err = IM_ERR_THREAD;
         return false;
@@ -1226,6 +1246,7 @@ bool im_stop_ind(IMManager *manager, const CMPIContext *ctx, IMError *err)
     manager->data = NULL;
     /* TODO: properly detach the thread using CBAttachThread(), needs to be called from the thread */
     manager->ctx_manage = NULL;
+    manager->cancelled = false;
 
     return true;
 }
diff --git a/src/indmanager/ind_manager.h b/src/indmanager/ind_manager.h
index 733cf7a..a684ac6 100644
--- a/src/indmanager/ind_manager.h
+++ b/src/indmanager/ind_manager.h
@@ -136,6 +136,7 @@ struct _IMManager {
     IMIndType type;
     bool running;
     bool polling;
+    bool cancelled;
     const CMPIBroker *broker;
     const CMPIContext *ctx_main; /* main thread */
     CMPIContext *ctx_manage; /* manage thread */
@@ -163,6 +164,7 @@ typedef enum {
     IM_ERR_THREAD,           // some error on threading
     IM_ERR_CMPI_RC,          // CMPI status not OK
     IM_ERR_OP,               // bad or null object path
+    IM_ERR_CANCELLED,        // job has been cancelled
 } IMError;
 
 // Create manager with given properties and callbacks.