Fix threading bug in DNS-SD API (Issue #33)

Author: michaelrsweet

cups/dest.c

@@ -143,6 +143,7 @@ static size_t		cups_get_dests(const char *filename, const char *match_name, cons
 static char		*cups_make_string(ipp_attribute_t *attr, char *buffer, size_t bufsize);
 static bool		cups_name_cb(_cups_namedata_t *data, unsigned flags, cups_dest_t *dest);
 static void		cups_queue_name(char *name, const char *serviceName, size_t namesize);
+static void		dnssd_error_cb(void *cb_data, const char *message);
 
 
 /*
@@ -3149,7 +3150,7 @@ cups_enum_dests(
 
   gettimeofday(&curtime, NULL);
 
-  if ((dnssd = cupsDNSSDNew(NULL, NULL)) == NULL)
+  if ((dnssd = cupsDNSSDNew(dnssd_error_cb, NULL)) == NULL)
   {
     DEBUG_puts("1cups_enum_dests: Unable to create service browser, returning 0.");
 
@@ -3811,3 +3812,17 @@ cups_queue_name(
 
   *nameptr = '\0';
 }
+
+
+//
+// 'dnssd_error_cb()' - Report an error.
+//
+
+static void
+dnssd_error_cb(void       *cb_data,	// I - Callback data (unused)
+               const char *message)	// I - Message
+{
+  (void)cb_data;
+  _cupsSetError(IPP_STATUS_ERROR_INTERNAL, message, 0);
+}
+

cups/dnssd.c

@@ -1,7 +1,7 @@
 //
 // DNS-SD API functions for CUPS.
 //
-// Copyright © 2022 by OpenPrinting.
+// Copyright © 2022-2023 by OpenPrinting.
 //
 // Licensed under Apache License v2.0.  See the file "LICENSE" for more
 // information.
@@ -409,12 +409,14 @@ cupsDNSSDDelete(cups_dnssd_t *dnssd)	// I - DNS-SD context
 
 #ifdef HAVE_MDNSRESPONDER
   cupsThreadCancel(dnssd->monitor);
+  cupsThreadWait(dnssd->monitor);
   DNSServiceRefDeallocate(dnssd->ref);
 
 #elif _WIN32
 
 #else // HAVE_AVAHI
   cupsThreadCancel(dnssd->monitor);
+  cupsThreadWait(dnssd->monitor);
   avahi_simple_poll_free(dnssd->poll);
 #endif // HAVE_MDNSRESPONDER
 
@@ -562,7 +564,6 @@ cupsDNSSDNew(
     return (NULL);
   }
 
-  cupsThreadDetach(dnssd->monitor);
   DEBUG_printf(("2cupsDNSSDNew: dnssd->monitor=%p", (void *)dnssd->monitor));
 
 #elif _WIN32
@@ -603,7 +604,6 @@ cupsDNSSDNew(
     return (NULL);
   }
 
-  cupsThreadDetach(dnssd->monitor);
   DEBUG_printf(("2cupsDNSSDNew: dnssd->monitor=%p", (void *)dnssd->monitor));
 #endif // HAVE_MDNSRESPONDER
 
@@ -1631,9 +1631,21 @@ static void *				// O - Return value (always `NULL`)
 mdns_monitor(cups_dnssd_t *dnssd)	// I - DNS-SD context
 {
   DNSServiceErrorType	error;		// Current error
+  struct pollfd		polldata;	// Polling data
+
+  polldata.fd     = DNSServiceRefSockFD(dnssd->ref);
+  polldata.events = POLLERR | POLLHUP | POLLIN;
 
   for (;;)
   {
+#  ifndef _WIN32
+    if (poll(&polldata, 1, 1000) < 0 && errno != EINTR && errno != EAGAIN)
+      break;
+
+    if (!(polldata.revents & POLLIN))
+      continue;
+#  endif // !_WIN32
+
     if ((error = DNSServiceProcessResult(dnssd->ref)) != kDNSServiceErr_NoError)
     {
       report_error(dnssd, "Unable to read response from DNS-SD service: %s", mdns_strerror(error));

cups/testgetdests.c

@@ -40,7 +40,7 @@ main(int  argc,				// I - Number of command-line arguments
   if (argc == 2)
     count = strtoul(argv[1], NULL, 10);
   else
-    count = 1;
+    count = 5;
 
   // Run tests...
   while (count > 0)
@@ -51,7 +51,10 @@ main(int  argc,				// I - Number of command-line arguments
     gettimeofday(&end, NULL);
     secs = end.tv_sec - start.tv_sec + 0.000001 * (end.tv_usec - start.tv_usec);
 
-    testEndMessage(secs < 2.0, "%u printers in %.3f seconds", (unsigned)num_dests, secs);
+    if (cupsLastError() != IPP_STATUS_OK)
+      testEndMessage(false, "%s", cupsLastErrorString());
+    else
+      testEndMessage(secs < 2.0, "%u printers in %.3f seconds", (unsigned)num_dests, secs);
 
     cupsFreeDests(num_dests, dests);
 
@@ -61,5 +64,5 @@ main(int  argc,				// I - Number of command-line arguments
       sleep(1);
   }
 
-  return (0);
+  return (testsPassed ? 0 : 1);
 }

cups/thread.c

@@ -1,7 +1,7 @@
 //
 // Threading primitives for CUPS.
 //
-// Copyright © 2021-2022 by OpenPrinting.
+// Copyright © 2021-2023 by OpenPrinting.
 // Copyright © 2009-2018 by Apple Inc.
 //
 // Licensed under Apache License v2.0.  See the file "LICENSE" for more