Fix some Coverity-discovered locking issues in the subscriptions code.

diff --git a/scheduler/ipp.c b/scheduler/ipp.c
index 8691e2662a713a14da35b39e6f4e91987738f418..64db4b70fd688a8b10d32f304aab5560f76e53f9 100644 (file)
--- a/scheduler/ipp.c
+++ b/scheduler/ipp.c
@@ -3506,12 +3506,17 @@ static int                              /* O - 1 if OK, 0 if not */
 check_rss_recipient(
     const char *recipient)             /* I - Recipient URI */
 {
+  int                  i,              /* Looping var */
+                       scount;         /* Number of subscriptions */
   cupsd_subscription_t *sub;           /* Current subscription */
 
 
-  for (sub = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions);
-       sub;
-       sub = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
+  cupsRWLockRead(&SubscriptionsLock);
+
+  for (i = 0, scount = cupsArrayGetCount(Subscriptions); i < scount; i ++)
+  {
+    sub = (cupsd_subscription_t *)cupsArrayGetElement(Subscriptions, i);
+
     if (sub->recipient)
     {
      /*
@@ -3527,6 +3532,9 @@ check_rss_recipient(
       if (*r1 == *r2)
         return (0);
     }
+  }
+
+  cupsRWUnlock(&SubscriptionsLock);
 
   return (1);
 }
@@ -9211,14 +9219,14 @@ renew_subscription(
 
   sub->expire = sub->lease ? time(NULL) + sub->lease : 0;
 
-  cupsRWUnlock(&sub->lock);
-
   cupsdMarkDirty(CUPSD_DIRTY_SUBSCRIPTIONS);
 
   con->response->request.status.status_code = IPP_STATUS_OK;
 
   ippAddInteger(con->response, IPP_TAG_SUBSCRIPTION, IPP_TAG_INTEGER,
                 "notify-lease-duration", sub->lease);
+
+  cupsRWUnlock(&sub->lock);
 }
 
 

diff --git a/scheduler/subscriptions.c b/scheduler/subscriptions.c
index 610c2e8b72b915ec299eea68f79bc849b0ba726e..ef8ea0bd04a03f1acc76585dbea3ebf11a48927f 100644 (file)
--- a/scheduler/subscriptions.c
+++ b/scheduler/subscriptions.c
@@ -419,19 +419,23 @@ cupsdDeleteAllSubscriptions(void)
   cupsd_subscription_t *sub;           /* Subscription */
 
 
-  if (!Subscriptions)
-    return;
-
   cupsRWLockWrite(&SubscriptionsLock);
 
+  if (!Subscriptions)
+    goto done;
+
   for (sub = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions);
        sub;
        sub = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
+  {
     cupsdDeleteSubscription(sub, -1);
+  }
 
   cupsArrayDelete(Subscriptions);
   Subscriptions = NULL;
 
+  done:
+
   cupsRWUnlock(&SubscriptionsLock);
 }
 
@@ -642,20 +646,19 @@ cupsdExpireSubscriptions(
     cupsd_job_t            *job)               /* I - Job, if any */
 {
   cupsd_subscription_t *sub;           /* Current subscription */
-  int                  update;         /* Update subscriptions.conf? */
+  int                  update = 0;     /* Update subscriptions.conf? */
   time_t               curtime;        /* Current time */
 
 
+  cupsRWLockWrite(&SubscriptionsLock);
+
   if (cupsArrayCount(Subscriptions) == 0)
-    return;
+    goto done;
 
   curtime = time(NULL);
-  update  = 0;
 
   cupsdLogMessage(CUPSD_LOG_DEBUG, "Expiring subscriptions...");
 
-  cupsRWLockWrite(&SubscriptionsLock);
-
   for (sub = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions);
        sub;
        sub = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
@@ -673,6 +676,8 @@ cupsdExpireSubscriptions(
     }
   }
 
+  done:
+
   cupsRWUnlock(&SubscriptionsLock);
 
   if (update)
@@ -1207,9 +1212,12 @@ cupsdStopAllNotifiers(void)
   * Yes, kill any processes that are left...
   */
 
+  cupsRWLockWrite(&SubscriptionsLock);
+
   for (sub = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions);
        sub;
        sub = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
+  {
     if (sub->pid)
     {
       cupsdEndProcess(sub->pid, 0);
@@ -1217,6 +1225,9 @@ cupsdStopAllNotifiers(void)
       close(sub->pipe);
       sub->pipe = -1;
     }
+  }
+
+  cupsRWUnlock(&SubscriptionsLock);
 
  /*
   * Close the status pipes...
@@ -1387,7 +1398,7 @@ cupsd_send_notification(
       cupsdLogMessage(CUPSD_LOG_CRIT,
                      "Unable to allocate memory for subscription #%d!",
                      sub->id);
-      return;
+      goto done;
     }
   }
 
@@ -1475,6 +1486,8 @@ cupsd_send_notification(
 
   sub->next_event_id ++;
 
+  done:
+
   cupsRWUnlock(&sub->lock);
 }