Fix threading issues for subscriptions in cupsd (Issue #1235)

diff --git a/CHANGES.md b/CHANGES.md
index bf925c306f1e4895d1a7f6c705fdf0e67d557366..6409cb6938a6aa31b3bbbc3f91c06241e47cf312 100644 (file)
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -7,6 +7,7 @@ Changes in CUPS v2.4.13 (YYYY-MM-DD)
 
 - Fixed a memory leak in `httpClose` (Issue #1223)
 - Fixed missing commas in `ippCreateRequestedArray` (Issue #1234)
+- Fixed subscription threading issues in the scheduler (Issue #1235)
 
 
 Changes in CUPS v2.4.12 (2025-04-08)

diff --git a/cups/libcups2.def b/cups/libcups2.def
index da75298b3cb00aab479743319830e841a9720c0f..6e41d51b1121fe2c881d875b5b388495a6142e2e 100644 (file)
--- a/cups/libcups2.def
+++ b/cups/libcups2.def
@@ -36,6 +36,7 @@ _cupsMutexInit
 _cupsMutexLock
 _cupsMutexUnlock
 _cupsNextDelay
+_cupsRWDestroy
 _cupsRWInit
 _cupsRWLockRead
 _cupsRWLockWrite

diff --git a/cups/thread-private.h b/cups/thread-private.h
index 1b8b1067fe2430415e5fb2002dea8fb4268a7aa6..9c918db936fd724eba66b32a2ebbf6322ddf689e 100644 (file)
--- a/cups/thread-private.h
+++ b/cups/thread-private.h
@@ -1,9 +1,11 @@
 /*
  * Private threading definitions for CUPS.
  *
- * Copyright 2009-2017 by Apple Inc.
+ * Copyright © 2025 by OpenPrinting.
+ * Copyright © 2009-2017 by Apple Inc.
  *
- * Licensed under Apache License v2.0.  See the file "LICENSE" for more information.
+ * Licensed under Apache License v2.0.  See the file "LICENSE" for more
+ * information.
  */
 
 #ifndef _CUPS_THREAD_PRIVATE_H_
@@ -88,6 +90,7 @@ extern void   _cupsCondWait(_cups_cond_t *cond, _cups_mutex_t *mutex, double timeo
 extern void    _cupsMutexInit(_cups_mutex_t *mutex) _CUPS_PRIVATE;
 extern void    _cupsMutexLock(_cups_mutex_t *mutex) _CUPS_PRIVATE;
 extern void    _cupsMutexUnlock(_cups_mutex_t *mutex) _CUPS_PRIVATE;
+extern void    _cupsRWDestroy(_cups_rwlock_t *rwlock) _CUPS_PRIVATE;
 extern void    _cupsRWInit(_cups_rwlock_t *rwlock) _CUPS_PRIVATE;
 extern void    _cupsRWLockRead(_cups_rwlock_t *rwlock) _CUPS_PRIVATE;
 extern void    _cupsRWLockWrite(_cups_rwlock_t *rwlock) _CUPS_PRIVATE;

diff --git a/cups/thread.c b/cups/thread.c
index 066f8e2e8ea3c168f62e9b1c799da17a721c4e5f..57e3315adc9a8c4fb18018082777bba8156ff6cb 100644 (file)
--- a/cups/thread.c
+++ b/cups/thread.c
@@ -1,7 +1,7 @@
 /*
  * Threading primitives for CUPS.
  *
- * Copyright © 2020-2024 by OpenPrinting.
+ * Copyright © 2020-2025 by OpenPrinting.
  * Copyright © 2009-2018 by Apple Inc.
  *
  * Licensed under Apache License v2.0.  See the file "LICENSE" for more
@@ -103,6 +103,17 @@ _cupsMutexUnlock(_cups_mutex_t *mutex)     /* I - Mutex */
 }
 
 
+/*
+ * '_cupsRWDestroy()' - Destroy a reader/writer lock.
+ */
+
+void
+_cupsRWDestroy(_cups_rwlock_t *rwlock) /* I - Reader/writer lock */
+{
+  pthread_rwlock_destroy(rwlock);
+}
+
+
 /*
  * '_cupsRWInit()' - Initialize a reader/writer lock.
  */
@@ -290,6 +301,17 @@ _cupsMutexUnlock(_cups_mutex_t *mutex)     /* I - Mutex */
 }
 
 
+/*
+ * '_cupsRWDestroy()' - Destroy a reader/writer lock.
+ */
+
+void
+_cupsRWDestroy(_cups_rwlock_t *rwlock) /* I - Reader/writer lock */
+{
+  (void)rwlock;
+}
+
+
 /*
  * '_cupsRWInit()' - Initialize a reader/writer lock.
  */

diff --git a/scheduler/ipp.c b/scheduler/ipp.c
index 96ed025969e97b1194c511bf1454f3ff552df0d7..cc095b0e79802f104efc58a6b5983206b8fd710e 100644 (file)
--- a/scheduler/ipp.c
+++ b/scheduler/ipp.c
@@ -5063,9 +5063,9 @@ copy_subscription_attrs(
   const char           *name;          /* Current event name */
 
 
-  cupsdLogMessage(CUPSD_LOG_DEBUG2,
-                  "copy_subscription_attrs(con=%p, sub=%p, ra=%p, exclude=%p)",
-                 (void *)con, (void *)sub, (void *)ra, (void *)exclude);
+  cupsdLogMessage(CUPSD_LOG_DEBUG2, "copy_subscription_attrs(con=%p, sub=%p, ra=%p, exclude=%p)", (void *)con, (void *)sub, (void *)ra, (void *)exclude);
+
+  _cupsRWLockRead(&sub->lock);
 
  /*
   * Copy the subscription attributes to the response using the
@@ -5156,6 +5156,8 @@ copy_subscription_attrs(
   if (!ra || cupsArrayFind(ra, "notify-subscription-id"))
     ippAddInteger(con->response, IPP_TAG_SUBSCRIPTION, IPP_TAG_INTEGER,
                   "notify-subscription-id", sub->id);
+
+  _cupsRWUnlock(&sub->lock);
 }
 
 
@@ -5433,7 +5435,6 @@ create_local_bg_thread(
     goto finish_response;
   }
 
-  // TODO: Grab printer icon file...
   httpClose(http);
 
  /*
@@ -7940,7 +7941,9 @@ get_subscriptions(cupsd_client_t  *con,   /* I - Client connection */
                   ipp_attribute_t *uri)        /* I - Printer/job URI */
 {
   http_status_t                status;         /* Policy status */
-  int                  count;          /* Number of subscriptions */
+  int                  i,              /* Looping var */
+                       scount;         /* Total number of subscriptions */
+  int                  count;          /* Number of subscriptions returned */
   int                  limit;          /* Limit */
   cupsd_subscription_t *sub;           /* Subscription */
   cups_array_t         *ra;            /* Requested attributes array */
@@ -8064,9 +8067,12 @@ get_subscriptions(cupsd_client_t  *con,  /* I - Client connection */
   else
     username[0] = '\0';
 
-  for (sub = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions), count = 0;
-       sub;
-       sub = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
+  _cupsRWLockRead(&SubscriptionsLock);
+
+  for (i = 0, count = 0, scount = cupsArrayCount(Subscriptions); i < scount; i ++)
+  {
+    sub = (cupsd_subscription_t *)cupsArrayIndex(Subscriptions, i);
+
     if ((!printer || sub->dest == printer) && (!job || sub->job == job) &&
         (!username[0] || !_cups_strcasecmp(username, sub->owner)))
     {
@@ -8082,6 +8088,9 @@ get_subscriptions(cupsd_client_t  *con,   /* I - Client connection */
       if (limit && count >= limit)
         break;
     }
+  }
+
+  _cupsRWUnlock(&SubscriptionsLock);
 
   cupsArrayDelete(ra);
 
@@ -9455,6 +9464,8 @@ renew_subscription(
   * Renew the subscription...
   */
 
+  _cupsRWLockWrite(&sub->lock);
+
   lease = ippFindAttribute(con->request, "notify-lease-duration",
                            IPP_TAG_INTEGER);
 
@@ -9471,9 +9482,11 @@ 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_OK;
+  con->response->request.status.status_code = IPP_STATUS_OK;
 
   ippAddInteger(con->response, IPP_TAG_SUBSCRIPTION, IPP_TAG_INTEGER,
                 "notify-lease-duration", sub->lease);

diff --git a/scheduler/subscriptions.c b/scheduler/subscriptions.c
index eefa7f82e3313717431166063f549a221ed23855..8f094445a91c548a71d5fd938cc47af8cc6067be 100644 (file)
--- a/scheduler/subscriptions.c
+++ b/scheduler/subscriptions.c
@@ -1,7 +1,7 @@
 /*
  * Subscription routines for the CUPS scheduler.
  *
- * Copyright © 2020-2024 by OpenPrinting.
+ * Copyright © 2020-2025 by OpenPrinting.
  * Copyright © 2007-2019 by Apple Inc.
  * Copyright © 1997-2007 by Easy Software Products, all rights reserved.
  *
@@ -54,10 +54,12 @@ cupsdAddEvent(
     const char       *text,            /* I - Notification text */
     ...)                               /* I - Additional arguments as needed */
 {
+  int                  i,              /* Looping var */
+                       scount;         /* Number of subscriptions */
   va_list              ap;             /* Pointer to additional arguments */
   char                 ftext[1024];    /* Formatted text buffer */
   ipp_attribute_t      *attr;          /* Printer/job attribute */
-  cupsd_event_t                *temp;          /* New event pointer */
+  cupsd_event_t                *temp = NULL;   /* New event pointer */
   cupsd_subscription_t *sub;           /* Current subscription */
 
 
@@ -96,14 +98,16 @@ cupsdAddEvent(
   if (job && !dest)
     dest = cupsdFindPrinter(job->dest);
 
-  for (temp = NULL, sub = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions);
-       sub;
-       sub = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
+  _cupsRWLockRead(&SubscriptionsLock);
+
+  for (i = 0, scount = cupsArrayCount(Subscriptions); i < scount; i ++)
   {
    /*
     * Check if this subscription requires this event...
     */
 
+    sub = (cupsd_subscription_t *)cupsArrayIndex(Subscriptions, i);
+
     if ((sub->mask & event) != 0 && (sub->dest == dest || !sub->dest || sub->job == job))
     {
      /*
@@ -115,6 +119,7 @@ cupsdAddEvent(
        cupsdLogMessage(CUPSD_LOG_CRIT,
                        "Unable to allocate memory for event - %s",
                        strerror(errno));
+        _cupsRWUnlock(&SubscriptionsLock);
        return;
       }
 
@@ -239,6 +244,8 @@ cupsdAddEvent(
     }
   }
 
+  _cupsRWUnlock(&SubscriptionsLock);
+
   if (temp)
     cupsdMarkDirty(CUPSD_DIRTY_SUBSCRIPTIONS);
   else
@@ -264,9 +271,11 @@ cupsdAddSubscription(
   cupsdLogMessage(CUPSD_LOG_DEBUG,
                  "cupsdAddSubscription(mask=%x, dest=%p(%s), job=%p(%d), "
                  "uri=\"%s\")",
-                 mask, dest, dest ? dest->name : "", job, job ? job->id : 0,
+                 mask, (void *)dest, dest ? dest->name : "", (void *)job, job ? job->id : 0,
                  uri ? uri : "(null)");
 
+  _cupsRWLockWrite(&SubscriptionsLock);
+
   if (!Subscriptions)
     Subscriptions = cupsArrayNew((cups_array_func_t)cupsd_compare_subscriptions,
                                 NULL);
@@ -276,6 +285,7 @@ cupsdAddSubscription(
     cupsdLogMessage(CUPSD_LOG_CRIT,
                    "Unable to allocate memory for subscriptions - %s",
                    strerror(errno));
+    _cupsRWUnlock(&SubscriptionsLock);
     return (NULL);
   }
 
@@ -289,6 +299,7 @@ cupsdAddSubscription(
                    "cupsdAddSubscription: Reached MaxSubscriptions %d "
                    "(count=%d)", MaxSubscriptions,
                    cupsArrayCount(Subscriptions));
+    _cupsRWUnlock(&SubscriptionsLock);
     return (NULL);
   }
 
@@ -309,6 +320,7 @@ cupsdAddSubscription(
                      "cupsdAddSubscription: Reached MaxSubscriptionsPerJob %d "
                      "for job #%d (count=%d)", MaxSubscriptionsPerJob,
                      job->id, count);
+      _cupsRWUnlock(&SubscriptionsLock);
       return (NULL);
     }
   }
@@ -330,6 +342,7 @@ cupsdAddSubscription(
                      "cupsdAddSubscription: Reached "
                      "MaxSubscriptionsPerPrinter %d for %s (count=%d)",
                      MaxSubscriptionsPerPrinter, dest->name, count);
+      _cupsRWUnlock(&SubscriptionsLock);
       return (NULL);
     }
   }
@@ -343,9 +356,12 @@ cupsdAddSubscription(
     cupsdLogMessage(CUPSD_LOG_CRIT,
                    "Unable to allocate memory for subscription object - %s",
                    strerror(errno));
+    _cupsRWUnlock(&SubscriptionsLock);
     return (NULL);
   }
 
+  _cupsRWInit(&temp->lock);
+
  /*
   * Fill in common data...
   */
@@ -379,6 +395,8 @@ cupsdAddSubscription(
 
   cupsArrayAdd(Subscriptions, temp);
 
+  _cupsRWUnlock(&SubscriptionsLock);
+
  /*
   * For RSS subscriptions, run the notifier immediately...
   */
@@ -403,6 +421,8 @@ cupsdDeleteAllSubscriptions(void)
   if (!Subscriptions)
     return;
 
+  _cupsRWLockWrite(&SubscriptionsLock);
+
   for (sub = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions);
        sub;
        sub = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
@@ -410,6 +430,8 @@ cupsdDeleteAllSubscriptions(void)
 
   cupsArrayDelete(Subscriptions);
   Subscriptions = NULL;
+
+  _cupsRWUnlock(&SubscriptionsLock);
 }
 
 
@@ -433,12 +455,18 @@ cupsdDeleteSubscription(
   * Remove subscription from array...
   */
 
+  _cupsRWLockWrite(&SubscriptionsLock);
+
   cupsArrayRemove(Subscriptions, sub);
 
+  _cupsRWUnlock(&SubscriptionsLock);
+
  /*
   * Free memory...
   */
 
+  _cupsRWDestroy(&sub->lock);
+
   cupsdClearString(&(sub->owner));
   cupsdClearString(&(sub->recipient));
 
@@ -650,12 +678,18 @@ cupsdExpireSubscriptions(
 cupsd_subscription_t *                 /* O - Subscription object */
 cupsdFindSubscription(int id)          /* I - Subscription ID */
 {
-  cupsd_subscription_t sub;            /* Subscription template */
+  cupsd_subscription_t key,            /* Subscription key */
+                       *sub;           /* Matching subscription */
+
+  key.id = id;
+
+  _cupsRWLockRead(&SubscriptionsLock);
 
+  sub = (cupsd_subscription_t *)cupsArrayFind(Subscriptions, &sub);
 
-  sub.id = id;
+  _cupsRWUnlock(&SubscriptionsLock);
 
-  return ((cupsd_subscription_t *)cupsArrayFind(Subscriptions, &sub));
+  return (sub);
 }
 
 
@@ -1019,7 +1053,8 @@ cupsdLoadAllSubscriptions(void)
 void
 cupsdSaveAllSubscriptions(void)
 {
-  int                  i;              /* Looping var */
+  int                  i, j,           /* Looping vars */
+                       scount;         /* Number of subscriptions */
   cups_file_t          *fp;            /* subscriptions.conf file */
   char                 filename[1024]; /* subscriptions.conf filename */
   cupsd_subscription_t *sub;           /* Current subscription */
@@ -1052,10 +1087,12 @@ cupsdSaveAllSubscriptions(void)
   * Write every subscription known to the system...
   */
 
-  for (sub = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions);
-       sub;
-       sub = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
+  _cupsRWLockRead(&SubscriptionsLock);
+
+  for (i = 0, scount = cupsArrayCount(Subscriptions); i < scount; i ++)
   {
+    sub = (cupsd_subscription_t *)cupsArrayIndex(Subscriptions, i);
+
     cupsFilePrintf(fp, "<Subscription %d>\n", sub->id);
 
     if ((name = cupsdEventName((cupsd_eventmask_t)sub->mask)) != NULL)
@@ -1094,29 +1131,29 @@ cupsdSaveAllSubscriptions(void)
     {
       cupsFilePuts(fp, "UserData ");
 
-      for (i = 0, hex = 0; i < sub->user_data_len; i ++)
+      for (j = 0, hex = 0; j < sub->user_data_len; j ++)
       {
-       if (sub->user_data[i] < ' ' ||
-           sub->user_data[i] > 0x7f ||
-           sub->user_data[i] == '<')
+       if (sub->user_data[j] < ' ' ||
+           sub->user_data[j] > 0x7f ||
+           sub->user_data[j] == '<')
        {
          if (!hex)
          {
-           cupsFilePrintf(fp, "<%02X", sub->user_data[i]);
+           cupsFilePrintf(fp, "<%02X", sub->user_data[j]);
            hex = 1;
          }
          else
-           cupsFilePrintf(fp, "%02X", sub->user_data[i]);
+           cupsFilePrintf(fp, "%02X", sub->user_data[j]);
        }
        else
        {
          if (hex)
          {
-           cupsFilePrintf(fp, ">%c", sub->user_data[i]);
+           cupsFilePrintf(fp, ">%c", sub->user_data[j]);
            hex = 0;
          }
          else
-           cupsFilePutChar(fp, sub->user_data[i]);
+           cupsFilePutChar(fp, sub->user_data[j]);
        }
       }
 
@@ -1134,6 +1171,8 @@ cupsdSaveAllSubscriptions(void)
     cupsFilePuts(fp, "</Subscription>\n");
   }
 
+  _cupsRWUnlock(&SubscriptionsLock);
+
   cupsdCloseCreatedConfFile(fp, filename);
 }
 
@@ -1325,6 +1364,8 @@ cupsd_send_notification(
   * Allocate the events array as needed...
   */
 
+  _cupsRWLockWrite(&sub->lock);
+
   if (!sub->events)
   {
     sub->events = cupsArrayNew3((cups_array_func_t)NULL, NULL,
@@ -1424,6 +1465,8 @@ cupsd_send_notification(
   */
 
   sub->next_event_id ++;
+
+  _cupsRWUnlock(&sub->lock);
 }
 
 

diff --git a/scheduler/subscriptions.h b/scheduler/subscriptions.h
index bc2a7dcb7018b016c47c19ba949b2b0464fb6d1d..44ea943417a256ec9664d83844f5c5013192b29b 100644 (file)
--- a/scheduler/subscriptions.h
+++ b/scheduler/subscriptions.h
@@ -1,11 +1,12 @@
 /*
  * Subscription definitions for the CUPS scheduler.
  *
- * Copyright © 2020-2024 by OpenPrinting.
- * Copyright 2007-2010 by Apple Inc.
- * Copyright 1997-2007 by Easy Software Products, all rights reserved.
+ * Copyright © 2020-2025 by OpenPrinting.
+ * Copyright © 2007-2010 by Apple Inc.
+ * Copyright © 1997-2007 by Easy Software Products, all rights reserved.
  *
- * Licensed under Apache License v2.0.  See the file "LICENSE" for more information.
+ * Licensed under Apache License v2.0.  See the file "LICENSE" for more
+ * information.
  */
 
 /*
@@ -79,6 +80,7 @@ typedef struct cupsd_event_s          /**** Event structure ****/
 
 typedef struct cupsd_subscription_s    /**** Subscription structure ****/
 {
+  _cups_rwlock_t       lock;           /* Reader/writer lock */
   int                  id;             /* subscription-id */
   unsigned             mask;           /* Event mask */
   char                 *owner;         /* notify-subscriber-user-name */
@@ -104,27 +106,30 @@ typedef struct cupsd_subscription_s       /**** Subscription structure ****/
  * Globals...
  */
 
-VAR int                MaxSubscriptions VALUE(100),
+VAR int                MaxSubscriptions        VALUE(100),
                                        /* Overall subscription limit */
-               MaxSubscriptionsPerJob VALUE(0),
+               MaxSubscriptionsPerJob  VALUE(0),
                                        /* Per-job subscription limit */
                MaxSubscriptionsPerPrinter VALUE(0),
                                        /* Per-printer subscription limit */
-               MaxSubscriptionsPerUser VALUE(0),
+               MaxSubscriptionsPerUser VALUE(0),
                                        /* Per-user subscription limit */
-               NextSubscriptionId VALUE(1),
+               NextSubscriptionId      VALUE(1),
                                        /* Next subscription ID */
-               DefaultLeaseDuration VALUE(86400),
+               DefaultLeaseDuration    VALUE(86400),
                                        /* Default notify-lease-duration */
-               MaxLeaseDuration VALUE(0);
+               MaxLeaseDuration        VALUE(0);
                                        /* Maximum notify-lease-duration */
-VAR cups_array_t *Subscriptions VALUE(NULL);
+VAR cups_array_t *Subscriptions                VALUE(NULL);
                                        /* Active subscriptions */
-
-VAR int                MaxEvents VALUE(100);   /* Maximum number of events */
-
-VAR unsigned   LastEvent VALUE(0);     /* Last event(s) processed */
-VAR int                NotifierPipes[2] VALUE2(-1, -1);
+VAR _cups_rwlock_t SubscriptionsLock   VALUE(_CUPS_RWLOCK_INITIALIZER);
+                                       /* Reader/writer lock for subscriptions */
+VAR int                MaxEvents               VALUE(100);
+                                       /* Maximum number of events */
+
+VAR unsigned   LastEvent               VALUE(0);
+                                       /* Last event(s) processed */
+VAR int                NotifierPipes[2]        VALUE2(-1, -1);
                                        /* Pipes for notifier error/debug output */
 VAR cupsd_statbuf_t *NotifierStatusBuffer VALUE(NULL);
                                        /* Status buffer for pipes */