diff --git a/CHANGES.md b/CHANGES.md index bf925c306..6409cb693 100644 --- 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 da75298b3..6e41d51b1 100644 --- 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 1b8b1067f..9c918db93 100644 --- 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 066f8e2e8..57e3315ad 100644 --- 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 96ed02596..cc095b0e7 100644 --- 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 eefa7f82e..8f094445a 100644 --- 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, "\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, "\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 bc2a7dcb7..44ea94341 100644 --- 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 */