account: Start watching right upon indication subscription

This is an attempt to get rid of most probable race conditions by creating
inotify watches outside the watcher thread and making them persistent for
the whole time the indication subscription is active. There's a certain
amount of time when watching if off due to gather part of the indication
manager. Leaving a watch fd open with active watches allows us to queue
events that we process next time the watcher is called (once gather part
is finished).

4 files changed, 90 insertions, 41 deletions

diff --git a/src/account/LMI_AccountInstanceCreationIndicationProvider.c b/src/account/LMI_AccountInstanceCreationIndicationProvider.c
index 06fe6e2..383bb98 100644
--- a/src/account/LMI_AccountInstanceCreationIndicationProvider.c
+++ b/src/account/LMI_AccountInstanceCreationIndicationProvider.c
@@ -32,11 +32,17 @@ static const CMPIBroker* _cb = NULL;
 
 static IMManager *im = NULL;
 static IMError im_err = IM_ERR_OK;
+static AccountIndication ai;
+
+static bool creation_watcher(void **data)
+{
+    return watcher(&ai, data);
+}
 
 static void LMI_AccountInstanceCreationIndicationInitialize(const CMPIContext *ctx)
 {
     lmi_init(provider_name, _cb, ctx, provider_config_defaults);
-    im = im_create_manager(NULL, filter_checker, true, watcher,
+    im = im_create_manager(NULL, filter_checker, true, creation_watcher,
                            IM_IND_CREATION, _cb, &im_err);
 }
 
@@ -108,7 +114,8 @@ static CMPIStatus LMI_AccountInstanceCreationIndicationEnableIndications(
     CMPIIndicationMI* mi,
     const CMPIContext* cc)
 {
-    if (!im_start_ind(im, cc, &im_err)) {
+    if (!watcher_init(&ai) ||
+        !im_start_ind(im, cc, &im_err)) {
         CMReturn(CMPI_RC_ERR_FAILED);
     }
     CMReturn(CMPI_RC_OK);
@@ -118,6 +125,7 @@ static CMPIStatus LMI_AccountInstanceCreationIndicationDisableIndications(
     CMPIIndicationMI* mi,
     const CMPIContext* cc)
 {
+    watcher_destroy(&ai);
     if (!im_stop_ind(im, cc, &im_err)) {
         CMReturn(CMPI_RC_ERR_FAILED);
     }
diff --git a/src/account/LMI_AccountInstanceDeletionIndicationProvider.c b/src/account/LMI_AccountInstanceDeletionIndicationProvider.c
index 81a0e18..f0de0c6 100644
--- a/src/account/LMI_AccountInstanceDeletionIndicationProvider.c
+++ b/src/account/LMI_AccountInstanceDeletionIndicationProvider.c
@@ -35,11 +35,17 @@ static const CMPIBroker* _cb = NULL;
 
 static IMManager *im = NULL;
 static IMError im_err = IM_ERR_OK;
+static AccountIndication ai;
+
+static bool deletion_watcher(void **data)
+{
+    return watcher(&ai, data);
+}
 
 static void LMI_AccountInstanceDeletionIndicationInitialize(const CMPIContext *ctx)
 {
     lmi_init(provider_name, _cb, ctx, provider_config_defaults);
-    im = im_create_manager(NULL, filter_checker, true, watcher,
+    im = im_create_manager(NULL, filter_checker, true, deletion_watcher,
                            IM_IND_DELETION, _cb, &im_err);
 }
 
@@ -111,7 +117,8 @@ static CMPIStatus LMI_AccountInstanceDeletionIndicationEnableIndications(
     CMPIIndicationMI* mi,
     const CMPIContext* cc)
 {
-    if (!im_start_ind(im, cc, &im_err)) {
+    if (!watcher_init(&ai) ||
+        !im_start_ind(im, cc, &im_err)) {
         CMReturn(CMPI_RC_ERR_FAILED);
     }
     CMReturn(CMPI_RC_OK);
@@ -121,6 +128,7 @@ static CMPIStatus LMI_AccountInstanceDeletionIndicationDisableIndications(
     CMPIIndicationMI* mi,
     const CMPIContext* cc)
 {
+    watcher_destroy(&ai);
     if (!im_stop_ind(im, cc, &im_err)) {
         CMReturn(CMPI_RC_ERR_FAILED);
     }
diff --git a/src/account/indication_common.c b/src/account/indication_common.c
index 70ce77b..22391e3 100644
--- a/src/account/indication_common.c
+++ b/src/account/indication_common.c
@@ -29,9 +29,10 @@
 #include <unistd.h>
 #include <sys/inotify.h>
 #include <limits.h>
+#include <errno.h>
 
 #include "indication_common.h"
-
+#include <globals.h>
 
 #include "LMI_Account.h"
 #include "LMI_Group.h"
@@ -47,7 +48,6 @@ static const char* allowed_classes[] = {
 #define PASSWD_FILE "/etc/passwd"
 #define GROUP_FILE "/etc/group"
 
-
 bool filter_checker(const CMPISelectExp *filter)
 {
     /*
@@ -114,25 +114,55 @@ static int timecmp(struct timespec a, struct timespec b)
         goto bail;\
     }\
 
-bool watcher(void **data)
+bool watcher_init(AccountIndication *ind)
 {
-    struct timespec last_pwd = get_last_mod(PASSWD_FILE);
-    struct timespec last_grp = get_last_mod(GROUP_FILE);
-    int fd = inotify_init();
+    ind->wd_pwd = 0;
+    ind->wd_grp = 0;
+    ind->inotify_fd = inotify_init();
 
-    if (fd < 0) {
+    if (ind->inotify_fd < 0)
         return false;
+
+    /* Get initial timestamps, at the beginning of watching. */
+    ind->last_pwd = get_last_mod(PASSWD_FILE);
+    ind->last_grp = get_last_mod(GROUP_FILE);
+
+    ADD_WATCH(ind->inotify_fd, ind->wd_pwd, PASSWD_FILE);
+    ADD_WATCH(ind->inotify_fd, ind->wd_grp, GROUP_FILE);
+
+    return true;
+
+bail:
+    watcher_destroy(ind);
+    return false;
+}
+
+void watcher_destroy(AccountIndication *ind)
+{
+    if (ind->inotify_fd >= 0) {
+        if (ind->wd_pwd > 0)
+            inotify_rm_watch(ind->inotify_fd, ind->wd_pwd);
+        if (ind->wd_grp > 0)
+            inotify_rm_watch(ind->inotify_fd, ind->wd_grp);
+        close(ind->inotify_fd);
+        ind->inotify_fd = -1;
     }
+}
+
+bool watcher(AccountIndication *ind, void **data)
+{
+    struct timespec curr_pwd, curr_grp;
     char buffer[BUF_LEN];
 
-    int wd_pwd, wd_grp = 0;
-    ADD_WATCH(fd, wd_pwd, PASSWD_FILE);
-    ADD_WATCH(fd, wd_grp, GROUP_FILE);
+    if (ind->inotify_fd < 0)
+        return false;
 
     do {
         int len = 0, i = 0;
-        if ((len = read(fd, buffer, BUF_LEN)) < 0) {
-            close(fd);
+        if ((len = read(ind->inotify_fd, buffer, BUF_LEN)) < 0) {
+            warn("account watcher: error reading from inotify fd: %s", strerror(errno));
+            watcher_destroy(ind);
+            watcher_init(ind);
             return false;
         }
         while (i < len) {
@@ -142,26 +172,34 @@ bool watcher(void **data)
                 case IN_CLOSE_WRITE:
                 case IN_DELETE:
                 case IN_DELETE_SELF:
-                    if (event->wd == wd_grp) {
-                        if (timecmp(last_grp, get_last_mod(GROUP_FILE)) == -1) {
-                            goto out;
+                    if (event->wd == ind->wd_grp) {
+                        curr_grp = get_last_mod(GROUP_FILE);
+                        if (timecmp(ind->last_grp, curr_grp) == -1) {
+                            ind->last_grp = curr_grp;
+                            return true;
                         }
                     } else {
-                        if (timecmp(last_pwd, get_last_mod(PASSWD_FILE)) == -1) {
-                            goto out;
+                        curr_pwd = get_last_mod(GROUP_FILE);
+                        if (timecmp(ind->last_pwd, curr_pwd) == -1) {
+                            ind->last_pwd = curr_pwd;
+                            return true;
                         }
                     }
                     break;
                 case IN_IGNORED:
-                    if (event->wd == wd_grp) {
-                        ADD_WATCH(fd, wd_grp, GROUP_FILE);
-                        if (timecmp(last_grp, get_last_mod(GROUP_FILE)) == -1) {
-                            goto out;
+                    if (event->wd == ind->wd_grp) {
+                        ADD_WATCH(ind->inotify_fd, ind->wd_grp, GROUP_FILE);
+                        curr_grp = get_last_mod(GROUP_FILE);
+                        if (timecmp(ind->last_grp, curr_grp) == -1) {
+                            ind->last_grp = curr_grp;
+                            return true;
                         }
                     } else {
-                        ADD_WATCH(fd, wd_pwd, PASSWD_FILE);
-                        if (timecmp(last_pwd, get_last_mod(PASSWD_FILE)) == -1) {
-                            goto out;
+                        ADD_WATCH(ind->inotify_fd, ind->wd_pwd, PASSWD_FILE);
+                        curr_pwd = get_last_mod(GROUP_FILE);
+                        if (timecmp(ind->last_pwd, curr_pwd) == -1) {
+                            ind->last_pwd = curr_pwd;
+                            return true;
                         }
                     }
                     break;
@@ -170,18 +208,6 @@ bool watcher(void **data)
         }
     } while (1);
 
-out:
-    inotify_rm_watch(fd, wd_pwd);
-    inotify_rm_watch(fd, wd_grp);
-    close(fd);
-    return true;
 bail:
-    if (wd_pwd > 0) {
-        inotify_rm_watch(fd, wd_pwd);
-    }
-    if (wd_grp > 0) {
-        inotify_rm_watch(fd, wd_grp);
-    }
-    close(fd);
     return false;
 }
diff --git a/src/account/indication_common.h b/src/account/indication_common.h
index d2727dc..6b7bc6d 100644
--- a/src/account/indication_common.h
+++ b/src/account/indication_common.h
@@ -18,6 +18,13 @@
  * Authors: Roman Rakus <rrakus@redhat.com>
  */
 
+typedef struct {
+    int wd_pwd, wd_grp;
+    int inotify_fd;
+    struct timespec last_pwd, last_grp;
+} AccountIndication;
 
 bool filter_checker(const CMPISelectExp *filter);
-bool watcher(void **data);
+bool watcher_init(AccountIndication *ind);
+bool watcher(AccountIndication *ind, void **data);
+void watcher_destroy(AccountIndication *ind);