s3-printing: reload shares after pcap cache fill

Since commit eada8f8a, updates to the cups pcap cache are performed
asynchronously - cups_cache_reload() forks a child process to request
cups printer information and notify the parent smbd on completion.

Currently printer shares are reloaded immediately following the call to
cups_cache_reload(), this occurs prior to smbd receiving new cups pcap
information from the child process. Such behaviour can result in stale
print shares as outlined in bug 7836.

This fix ensures print shares are only reloaded after new pcap data has
been received.

Pair-Programmed-With: Lars Müller <lars@samba.org>

4 files changed, 52 insertions, 15 deletions

diff --git a/source3/printing/load.c b/source3/printing/load.c
index 4f1bb88a99..66c3ffd22c 100644
--- a/source3/printing/load.c
+++ b/source3/printing/load.c
@@ -54,14 +54,12 @@ static void add_auto_printers(void)
 }
 
 /***************************************************************************
-load automatic printer services
+load automatic printer services from pre-populated pcap cache
 ***************************************************************************/
 void load_printers(struct tevent_context *ev,
 		   struct messaging_context *msg_ctx)
 {
-	if (!pcap_cache_loaded()) {
-		pcap_cache_reload(ev, msg_ctx);
-	}
+	SMB_ASSERT(pcap_cache_loaded());
 
 	add_auto_printers();
 
diff --git a/source3/printing/pcap.c b/source3/printing/pcap.c
index 1b8f46d8e7..be267bd116 100644
--- a/source3/printing/pcap.c
+++ b/source3/printing/pcap.c
@@ -107,11 +107,14 @@ void pcap_cache_replace(const struct pcap_cache *pcache)
 }
 
 void pcap_cache_reload(struct tevent_context *ev,
-		       struct messaging_context *msg_ctx)
+		       struct messaging_context *msg_ctx,
+		       void (*post_cache_fill_fn)(struct tevent_context *,
+						  struct messaging_context *))
 {
 	const char *pcap_name = lp_printcapname();
 	bool pcap_reloaded = False;
 	NTSTATUS status;
+	bool post_cache_fill_fn_handled = false;
 
 	DEBUG(3, ("reloading printcap cache\n"));
 
@@ -135,7 +138,13 @@ void pcap_cache_reload(struct tevent_context *ev,
 
 #ifdef HAVE_CUPS
 	if (strequal(pcap_name, "cups")) {
-		pcap_reloaded = cups_cache_reload(ev, msg_ctx);
+		pcap_reloaded = cups_cache_reload(ev, msg_ctx,
+						  post_cache_fill_fn);
+		/*
+		 * cups_cache_reload() is async and calls post_cache_fill_fn()
+		 * on successful completion
+		 */
+		post_cache_fill_fn_handled = true;
 		goto done;
 	}
 #endif
@@ -174,6 +183,10 @@ done:
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(0, ("Failed to cleanup printer list!\n"));
 		}
+		if ((post_cache_fill_fn_handled == false)
+		 && (post_cache_fill_fn != NULL)) {
+			post_cache_fill_fn(ev, msg_ctx);
+		}
 	}
 
 	return;
diff --git a/source3/printing/pcap.h b/source3/printing/pcap.h
index 67f36d6598..7f8f7d2baf 100644
--- a/source3/printing/pcap.h
+++ b/source3/printing/pcap.h
@@ -35,7 +35,9 @@ bool aix_cache_reload(void);
 /* The following definitions come from printing/print_cups.c  */
 
 bool cups_cache_reload(struct tevent_context *ev,
-		       struct messaging_context *msg_ctx);
+		       struct messaging_context *msg_ctx,
+		       void (*post_cache_fill_fn)(struct tevent_context *,
+						  struct messaging_context *));
 bool cups_pull_comment_location(TALLOC_CTX *mem_ctx,
 				const char *printername,
 				char **comment,
diff --git a/source3/printing/print_cups.c b/source3/printing/print_cups.c
index a85fba8997..8c023ddb30 100644
--- a/source3/printing/print_cups.c
+++ b/source3/printing/print_cups.c
@@ -449,13 +449,22 @@ static bool cups_pcap_load_async(struct tevent_context *ev,
 	_exit(0);
 }
 
+struct cups_async_cb_args {
+	int pipe_fd;
+	struct event_context *event_ctx;
+	struct messaging_context *msg_ctx;
+	void (*post_cache_fill_fn)(struct event_context *,
+				   struct messaging_context *);
+};
+
 static void cups_async_callback(struct event_context *event_ctx,
 				struct fd_event *event,
 				uint16 flags,
 				void *p)
 {
 	TALLOC_CTX *frame = talloc_stackframe();
-	int fd = *(int *)p;
+	struct cups_async_cb_args *cb_args = (struct cups_async_cb_args *)p;
+	int fd = cb_args->pipe_fd;
 	struct pcap_cache *tmp_pcap_cache = NULL;
 
 	DEBUG(5,("cups_async_callback: callback received for printer data. "
@@ -549,28 +558,43 @@ static void cups_async_callback(struct event_context *event_ctx,
 
 		/* And the systemwide pcap cache. */
 		pcap_cache_replace(local_pcap_copy);
+
+		/* Caller may have requested post cache fill callback */
+		if (cb_args->post_cache_fill_fn != NULL) {
+			cb_args->post_cache_fill_fn(cb_args->event_ctx,
+						    cb_args->msg_ctx);
+		}
 	} else {
 		DEBUG(2,("cups_async_callback: failed to read a new "
 			"printer list\n"));
 	}
 	close(fd);
-	TALLOC_FREE(p);
+	TALLOC_FREE(cb_args);
 	TALLOC_FREE(cache_fd_event);
 }
 
 bool cups_cache_reload(struct tevent_context *ev,
-		       struct messaging_context *msg_ctx)
+		       struct messaging_context *msg_ctx,
+		       void (*post_cache_fill_fn)(struct tevent_context *,
+						  struct messaging_context *))
 {
-	int *p_pipe_fd = TALLOC_P(NULL, int);
+	struct cups_async_cb_args *cb_args;
+	int *p_pipe_fd;
 
-	if (!p_pipe_fd) {
+	cb_args = TALLOC_P(NULL, struct cups_async_cb_args);
+	if (cb_args == NULL) {
 		return false;
 	}
 
+	cb_args->post_cache_fill_fn = post_cache_fill_fn;
+	cb_args->event_ctx = ev;
+	cb_args->msg_ctx = msg_ctx;
+	p_pipe_fd = &cb_args->pipe_fd;
 	*p_pipe_fd = -1;
 
 	/* Set up an async refresh. */
 	if (!cups_pcap_load_async(ev, msg_ctx, p_pipe_fd)) {
+		talloc_free(cb_args);
 		return false;
 	}
 	if (!local_pcap_copy) {
@@ -582,7 +606,7 @@ bool cups_cache_reload(struct tevent_context *ev,
 
 		cups_async_callback(ev, NULL,
 					EVENT_FD_READ,
-					(void *)p_pipe_fd);
+					(void *)cb_args);
 		if (!local_pcap_copy) {
 			return false;
 		}
@@ -599,10 +623,10 @@ bool cups_cache_reload(struct tevent_context *ev,
 					NULL, *p_pipe_fd,
 					EVENT_FD_READ,
 					cups_async_callback,
-					(void *)p_pipe_fd);
+					(void *)cb_args);
 		if (!cache_fd_event) {
 			close(*p_pipe_fd);
-			TALLOC_FREE(p_pipe_fd);
+			TALLOC_FREE(cb_args);
 			return false;
 		}
 	}