Skip to content

Commit

Permalink
Fix some more threading issues (Issue #155, Issue #162)
Browse files Browse the repository at this point in the history
  • Loading branch information
@michaelrsweet
michaelrsweet committed Apr 13, 2021
1 parent 73778fa commit 1d55a26
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 40 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Changes in v1.0.3

- The Set-Printer-Attributes operation did not save changes to
"printer-contact-col".
- Fixed some more threading issues (Issue #155, Issue #162)
- Fixed bogus USB error reporting (Issue #156)
- Fixed testpappl on systems without Avahi running (Issue #159)
- Adding a printer now works for names with special characters (Issue #161)
Expand Down
57 changes: 39 additions & 18 deletions pappl/printer-accessors.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -567,20 +567,27 @@ papplPrinterIterateActiveJobs(
int limit) // I - Maximum jobs to iterate or `0` for no limit
{
pappl_job_t *job; // Current job
int count; // Number of jobs
int j, // Looping var
jcount, // Number of jobs
count; // Number of jobs iterated


if (!printer || !cb)
return;

pthread_rwlock_rdlock(&printer->rwlock);

for (job = (pappl_job_t *)cupsArrayIndex(printer->active_jobs, job_index - 1), count = 0; job; job = (pappl_job_t *)cupsArrayNext(printer->active_jobs), count ++)
// Note: Cannot use cupsArrayFirst/Last since other threads might be
// enumerating the active_jobs array.

if (limit <= 0)
limit = INT_MAX;

for (count = 0, j = job_index - 1, jcount = cupsArrayCount(printer->active_jobs); j < jcount && count < limit; j ++, count ++)
{
if (limit == 0 || count < limit)
(cb)(job, data);
else
break;
job = (pappl_job_t *)cupsArrayIndex(printer->active_jobs, j);

(cb)(job, data);
}

pthread_rwlock_unlock(&printer->rwlock);
Expand All @@ -607,20 +614,27 @@ papplPrinterIterateAllJobs(
int limit) // I - Maximum jobs to iterate, `0` for no limit
{
pappl_job_t *job; // Current job
int count; // Number of jobs
int j, // Looping var
jcount, // Number of jobs
count; // Number of jobs iterated


if (!printer || !cb)
return;

pthread_rwlock_rdlock(&printer->rwlock);

for (job = (pappl_job_t *)cupsArrayIndex(printer->all_jobs, job_index - 1), count = 0; job; job = (pappl_job_t *)cupsArrayNext(printer->all_jobs), count ++)
// Note: Cannot use cupsArrayFirst/Last since other threads might be
// enumerating the all_jobs array.

if (limit <= 0)
limit = INT_MAX;

for (count = 0, j = job_index - 1, jcount = cupsArrayCount(printer->all_jobs); j < jcount && count < limit; j ++, count ++)
{
if (limit == 0 || count < limit)
(cb)(job, data);
else
break;
job = (pappl_job_t *)cupsArrayIndex(printer->all_jobs, j);

(cb)(job, data);
}

pthread_rwlock_unlock(&printer->rwlock);
Expand Down Expand Up @@ -648,20 +662,27 @@ papplPrinterIterateCompletedJobs(
int limit) // I - Maximum jobs to iterate, `0` for no limit
{
pappl_job_t *job; // Current job
int count; // Number of jobs
int j, // Looping var
jcount, // Number of jobs
count; // Number of jobs iterated


if (!printer || !cb)
return;

pthread_rwlock_rdlock(&printer->rwlock);

for (job = (pappl_job_t *)cupsArrayIndex(printer->completed_jobs, job_index - 1), count = 0; job; job = (pappl_job_t *)cupsArrayNext(printer->completed_jobs), count ++)
// Note: Cannot use cupsArrayFirst/Last since other threads might be
// enumerating the completed_jobs array.

if (limit <= 0)
limit = INT_MAX;

for (count = 0, j = job_index - 1, jcount = cupsArrayCount(printer->completed_jobs); j < jcount && count < limit; j ++, count ++)
{
if (limit == 0 || count < limit)
(cb)(job, data);
else
break;
job = (pappl_job_t *)cupsArrayIndex(printer->completed_jobs, j);

(cb)(job, data);
}

pthread_rwlock_unlock(&printer->rwlock);
Expand Down
17 changes: 15 additions & 2 deletions pappl/printer-ipp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,10 +409,16 @@ _papplPrinterCopyAttributes(
if (!ra || cupsArrayFind(ra, "printer-strings-languages-supported"))
{
_pappl_resource_t *r; // Current resource
int rcount; // Number of resources

pthread_rwlock_rdlock(&printer->system->rwlock);
for (num_values = 0, r = (_pappl_resource_t *)cupsArrayFirst(printer->system->resources); r && num_values < (int)(sizeof(svalues) / sizeof(svalues[0])); r = (_pappl_resource_t *)cupsArrayNext(printer->system->resources))

// Cannot use cupsArrayFirst/Last since other threads might be iterating
// this array...
for (i = 0, num_values = 0, rcount = cupsArrayCount(printer->system->resources); i < rcount && num_values < (int)(sizeof(svalues) / sizeof(svalues[0])); i ++)
{
r = (_pappl_resource_t *)cupsArrayIndex(printer->system->resources, i);

if (r->language)
svalues[num_values ++] = r->language;
}
Expand All @@ -429,19 +435,26 @@ _papplPrinterCopyAttributes(
char baselang[3], // Base language
uri[1024]; // Strings file URI
_pappl_resource_t *r; // Current resource
int rcount; // Number of resources

strlcpy(baselang, lang, sizeof(baselang));

pthread_rwlock_rdlock(&printer->system->rwlock);
for (r = (_pappl_resource_t *)cupsArrayFirst(printer->system->resources); r; r = (_pappl_resource_t *)cupsArrayNext(printer->system->resources))

// Cannot use cupsArrayFirst/Last since other threads might be iterating
// this array...
for (i = 0, num_values = 0, rcount = cupsArrayCount(printer->system->resources); i < rcount; i ++)
{
r = (_pappl_resource_t *)cupsArrayIndex(printer->system->resources, i);

if (r->language && (!strcmp(r->language, lang) || !strcmp(r->language, baselang)))
{
httpAssembleURI(HTTP_URI_CODING_ALL, uri, sizeof(uri), webscheme, NULL, client->host_field, client->host_port, r->path);
ippAddString(client->response, IPP_TAG_PRINTER, IPP_TAG_URI, "printer-strings-uri", NULL, uri);
break;
}
}

pthread_rwlock_unlock(&printer->system->rwlock);
}

Expand Down
9 changes: 6 additions & 3 deletions pappl/printer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ papplPrinterCancelAllJobs(
pappl_job_t *job; // Job information


// Loop through all jobs and cancel them...
// Loop through all jobs and cancel them.
//
// Since we have a writer lock, it is safe to use cupsArrayFirst/Last...
pthread_rwlock_wrlock(&printer->rwlock);

for (job = (pappl_job_t *)cupsArrayFirst(printer->active_jobs); job; job = (pappl_job_t *)cupsArrayNext(printer->active_jobs))
Expand Down Expand Up @@ -710,10 +712,11 @@ _papplPrinterDelete(
snprintf(prefix, sizeof(prefix), "%s/", printer->uriname);
prefixlen = strlen(prefix);

// Note: System writer lock is already held when calling cupsArrayRemove
// for the system's printer object, so we don't need a separate lock here
// and can safely use cupsArrayFirst/Next...
for (r = (_pappl_resource_t *)cupsArrayFirst(printer->system->resources); r; r = (_pappl_resource_t *)cupsArrayNext(printer->system->resources))
{
// Note: System rwlock is already held when calling cupsArrayRemove for the
// system's printer object, so we don't need a separate lock here...
if (r->cbdata == printer || !strncmp(r->path, prefix, prefixlen))
cupsArrayRemove(printer->system->resources, r);
}
Expand Down
12 changes: 9 additions & 3 deletions pappl/system-accessors.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1084,15 +1084,21 @@ papplSystemIteratePrinters(
pappl_printer_cb_t cb, // I - Callback function
void *data) // I - Callback data
{
pappl_printer_t *printer; // Current printer
int i, // Looping var
count; // Number of printers


if (!system || !cb)
return;

// Loop through the printers.
//
// Note: Cannot use cupsArrayFirst/Last since other threads might be
// enumerating the printers array.

pthread_rwlock_rdlock(&system->rwlock);
for (printer = (pappl_printer_t *)cupsArrayFirst(system->printers); printer; printer = (pappl_printer_t *)cupsArrayNext(system->printers))
(cb)(printer, data);
for (i = 0, count = cupsArrayCount(system->printers); i < count; i ++)
(cb)((pappl_printer_t *)cupsArrayIndex(system->printers, i), data);
pthread_rwlock_unlock(&system->rwlock);
}

Expand Down
36 changes: 27 additions & 9 deletions pappl/system-loadsave.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,8 @@ papplSystemSaveState(
pappl_system_t *system, // I - System
const char *filename) // I - File to save
{
int i; // Looping var
int i, j, // Looping vars
count; // Number of printers
cups_file_t *fp; // Output file
pappl_printer_t *printer; // Current printer
pappl_job_t *job; // Current Job
Expand Down Expand Up @@ -424,14 +425,24 @@ papplSystemSaveState(
cupsFilePrintf(fp, "NextPrinterID %d\n", system->next_printer_id);
cupsFilePutConf(fp, "UUID", system->uuid);

for (printer = (pappl_printer_t *)cupsArrayFirst(system->printers); printer; printer = (pappl_printer_t *)cupsArrayNext(system->printers))
// Loop through the printers.
//
// Note: Cannot use cupsArrayFirst/Last since other threads might be
// enumerating the printers array.

for (i = 0, count = cupsArrayCount(system->printers); i < count; i ++)
{
int jcount; // Number of jobs
int num_options = 0;// Number of options
cups_option_t *options = NULL;// Options

printer = (pappl_printer_t *)cupsArrayIndex(system->printers, i);

if (printer->is_deleted)
continue;

pthread_rwlock_rdlock(&printer->rwlock);

num_options = cupsAddIntegerOption("id", printer->printer_id, num_options, &options);
num_options = cupsAddOption("name", printer->name, num_options, &options);
num_options = cupsAddOption("did", printer->device_id ? printer->device_id : "", num_options, &options);
Expand Down Expand Up @@ -469,14 +480,14 @@ papplSystemSaveState(

write_media_col(fp, "media-col-default", &printer->driver_data.media_default);

for (i = 0; i < printer->driver_data.num_source; i ++)
for (j = 0; j < printer->driver_data.num_source; j ++)
{
if (printer->driver_data.media_ready[i].size_name[0])
if (printer->driver_data.media_ready[j].size_name[0])
{
char name[128]; // Attribute name

snprintf(name, sizeof(name), "media-col-ready%d", i);
write_media_col(fp, name, printer->driver_data.media_ready + i);
snprintf(name, sizeof(name), "media-col-ready%d", j);
write_media_col(fp, name, printer->driver_data.media_ready + j);
}
}
if (printer->driver_data.orient_default)
Expand All @@ -499,19 +510,24 @@ papplSystemSaveState(
cupsFilePutConf(fp, "sides-default", _papplSidesString(printer->driver_data.sides_default));
if (printer->driver_data.x_default)
cupsFilePrintf(fp, "printer-resolution-default %dx%ddpi\n", printer->driver_data.x_default, printer->driver_data.y_default);
for (i = 0; i < printer->driver_data.num_vendor; i ++)
for (j = 0; j < printer->driver_data.num_vendor; j ++)
{
char defname[128], // xxx-default name
defvalue[1024]; // xxx-default value

snprintf(defname, sizeof(defname), "%s-default", printer->driver_data.vendor[i]);
snprintf(defname, sizeof(defname), "%s-default", printer->driver_data.vendor[j]);
ippAttributeString(ippFindAttribute(printer->driver_attrs, defname, IPP_TAG_ZERO), defvalue, sizeof(defvalue));

cupsFilePutConf(fp, defname, defvalue);
}

for (job = (pappl_job_t *)cupsArrayFirst(printer->all_jobs); job; job = (pappl_job_t *)cupsArrayNext(printer->all_jobs))
// Note: Cannot use cupsArrayFirst/Last since other threads might be
// enumerating the all_jobs array.

for (j = 0, jcount = cupsArrayCount(printer->all_jobs); j < jcount; j ++)
{
job = (pappl_job_t *)cupsArrayIndex(printer->all_jobs, j);

// Add basic job attributes...
num_options = 0;
num_options = cupsAddIntegerOption("id", job->job_id, num_options, &options);
Expand Down Expand Up @@ -565,6 +581,8 @@ papplSystemSaveState(
}

cupsFilePuts(fp, "</Printer>\n");

pthread_rwlock_unlock(&printer->rwlock);
}

pthread_rwlock_unlock(&system->rwlock);
Expand Down
18 changes: 13 additions & 5 deletions pappl/system.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,8 +487,10 @@ papplSystemRun(pappl_system_t *system) // I - System
_papplSystemRegisterDNSSDNoLock(system);

// Start up printers...
for (printer = (pappl_printer_t *)cupsArrayFirst(system->printers); printer; printer = (pappl_printer_t *)cupsArrayNext(system->printers))
for (i = 0, count = cupsArrayCount(system->printers); i < count; i ++)
{
printer = (pappl_printer_t *)cupsArrayIndex(system->printers, i);

// Advertise via DNS-SD as needed...
if (printer->dns_sd_name)
_papplPrinterRegisterDNSSDNoLock(printer);
Expand Down Expand Up @@ -584,8 +586,10 @@ papplSystemRun(pappl_system_t *system) // I - System
if (system->dns_sd_collision || force_dns_sd)
_papplSystemRegisterDNSSDNoLock(system);

for (printer = (pappl_printer_t *)cupsArrayFirst(system->printers); printer; printer = (pappl_printer_t *)cupsArrayNext(system->printers))
for (i = 0, count = cupsArrayCount(system->printers); i < count; i ++)
{
printer = (pappl_printer_t *)cupsArrayIndex(system->printers, i);

if (printer->dns_sd_collision || force_dns_sd)
_papplPrinterRegisterDNSSDNoLock(printer);
}
Expand Down Expand Up @@ -625,8 +629,10 @@ papplSystemRun(pappl_system_t *system) // I - System

// Otherwise shutdown immediately if there are no more active jobs...
pthread_rwlock_rdlock(&system->rwlock);
for (printer = (pappl_printer_t *)cupsArrayFirst(system->printers); printer; printer = (pappl_printer_t *)cupsArrayNext(system->printers))
for (i = 0, count = cupsArrayCount(system->printers); i < count; i ++)
{
printer = (pappl_printer_t *)cupsArrayIndex(system->printers, i);

pthread_rwlock_rdlock(&printer->rwlock);
jcount += cupsArrayCount(printer->active_jobs);
pthread_rwlock_unlock(&printer->rwlock);
Expand All @@ -650,9 +656,11 @@ papplSystemRun(pappl_system_t *system) // I - System
if (system->dns_sd_name)
_papplSystemUnregisterDNSSDNoLock(system);

for (printer = (pappl_printer_t *)cupsArrayFirst(system->printers); printer; printer = (pappl_printer_t *)cupsArrayNext(system->printers))
for (i = 0, count = cupsArrayCount(system->printers); i < count; i ++)
{
// Advertise via DNS-SD as needed...
printer = (pappl_printer_t *)cupsArrayIndex(system->printers, i);

// Remove advertising via DNS-SD as needed...
if (printer->dns_sd_name)
_papplPrinterUnregisterDNSSDNoLock(printer);
}
Expand Down

0 comments on commit 1d55a26

Please sign in to comment.