Convert the winbind parent->child communication to wb_reqtrans

6 files changed, 166 insertions, 239 deletions

diff --git a/source3/winbindd/winbindd.h b/source3/winbindd/winbindd.h
index 314643c4e1..b294b6a4bd 100644
--- a/source3/winbindd/winbindd.h
+++ b/source3/winbindd/winbindd.h
@@ -148,10 +148,11 @@ struct winbindd_child {
 	struct winbindd_domain *domain;
 	char *logfilename;
 
-	struct winbindd_fd_event event;
+	int sock;
+	struct tevent_queue *queue;
+
 	struct timed_event *lockout_policy_event;
 	struct timed_event *machine_password_change_event;
-	struct winbindd_async_request *requests;
 
 	const struct winbindd_child_dispatch_table *table;
 };
diff --git a/source3/winbindd/winbindd_async.c b/source3/winbindd/winbindd_async.c
index 9f0c6817d8..af1b67db9c 100644
--- a/source3/winbindd/winbindd_async.c
+++ b/source3/winbindd/winbindd_async.c
@@ -781,8 +781,6 @@ static void getsidaliases_recv(TALLOC_CTX *mem_ctx, bool success,
 		return;
 	}
 
-	SAFE_FREE(response->extra_data.data);
-
 	cont(private_data, True, sids, num_sids);
 }
 
@@ -923,8 +921,6 @@ static void gettoken_recvdomgroups(TALLOC_CTX *mem_ctx, bool success,
 		return;
 	}
 
-	SAFE_FREE(response->extra_data.data);
-
 	if (state->alias_domain == NULL) {
 		DEBUG(10, ("Don't expand domain local groups\n"));
 		state->cont(state->private_data, True, state->sids,
diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c
index 9a5098f661..53a5bc47a4 100644
--- a/source3/winbindd/winbindd_dual.c
+++ b/source3/winbindd/winbindd_dual.c
@@ -29,6 +29,7 @@
 
 #include "includes.h"
 #include "winbindd.h"
+#include "../../nsswitch/libwbclient/wbc_async.h"
 
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_WINBIND
@@ -85,254 +86,188 @@ static void child_read_request(struct winbindd_cli_state *state)
 }
 
 /*
- * Machinery for async requests sent to children. You set up a
- * winbindd_request, select a child to query, and issue a async_request
- * call. When the request is completed, the callback function you specified is
- * called back with the private pointer you gave to async_request.
+ * Do winbind child async request. This is not simply wb_simple_trans. We have
+ * to do the queueing ourselves because while a request is queued, the child
+ * might have crashed, and we have to re-fork it in the _trigger function.
  */
 
-struct winbindd_async_request {
-	struct winbindd_async_request *next, *prev;
-	TALLOC_CTX *mem_ctx;
+struct wb_child_request_state {
+	struct tevent_context *ev;
 	struct winbindd_child *child;
 	struct winbindd_request *request;
 	struct winbindd_response *response;
-	void (*continuation)(void *private_data, bool success);
-	struct timed_event *reply_timeout_event;
-	pid_t child_pid; /* pid of the child we're waiting on. Used to detect
-			    a restart of the child (child->pid != child_pid). */
-	void *private_data;
 };
 
-static void async_request_fail(struct winbindd_async_request *state);
-static void async_main_request_sent(void *private_data, bool success);
-static void async_request_sent(void *private_data, bool success);
-static void async_reply_recv(void *private_data, bool success);
-static void schedule_async_request(struct winbindd_child *child);
-
-void async_request(TALLOC_CTX *mem_ctx, struct winbindd_child *child,
-		   struct winbindd_request *request,
-		   struct winbindd_response *response,
-		   void (*continuation)(void *private_data, bool success),
-		   void *private_data)
-{
-	struct winbindd_async_request *state;
-
-	SMB_ASSERT(continuation != NULL);
+static bool fork_domain_child(struct winbindd_child *child);
 
-	DEBUG(10, ("Sending request to child pid %d (domain=%s)\n",
-		   (int)child->pid,
-		   (child->domain != NULL) ? child->domain->name : "''"));
+static void wb_child_request_trigger(struct tevent_req *req,
+					    void *private_data);
+static void wb_child_request_done(struct tevent_req *subreq);
 
-	state = TALLOC_P(mem_ctx, struct winbindd_async_request);
+struct tevent_req *wb_child_request_send(TALLOC_CTX *mem_ctx,
+					 struct tevent_context *ev,
+					 struct winbindd_child *child,
+					 struct winbindd_request *request)
+{
+	struct tevent_req *req;
+	struct wb_child_request_state *state;
 
-	if (state == NULL) {
-		DEBUG(0, ("talloc failed\n"));
-		continuation(private_data, False);
-		return;
+	req = tevent_req_create(mem_ctx, &state,
+				struct wb_child_request_state);
+	if (req == NULL) {
+		return NULL;
 	}
 
-	state->mem_ctx = mem_ctx;
+	state->ev = ev;
 	state->child = child;
-	state->reply_timeout_event = NULL;
 	state->request = request;
-	state->response = response;
-	state->continuation = continuation;
-	state->private_data = private_data;
-
-	DLIST_ADD_END(child->requests, state, struct winbindd_async_request *);
 
-	schedule_async_request(child);
-
-	return;
+	if (!tevent_queue_add(child->queue, ev, req,
+			      wb_child_request_trigger, NULL)) {
+		tevent_req_nomem(NULL, req);
+		return tevent_req_post(req, ev);
+	}
+	return req;
 }
 
-static void async_main_request_sent(void *private_data, bool success)
+static void wb_child_request_trigger(struct tevent_req *req,
+				     void *private_data)
 {
-	struct winbindd_async_request *state =
-		talloc_get_type_abort(private_data, struct winbindd_async_request);
+	struct wb_child_request_state *state = tevent_req_data(
+		req, struct wb_child_request_state);
+	struct tevent_req *subreq;
 
-	if (!success) {
-		DEBUG(5, ("Could not send async request\n"));
-		async_request_fail(state);
+	if ((state->child->pid == 0) && (!fork_domain_child(state->child))) {
+		tevent_req_error(req, errno);
 		return;
 	}
 
-	if (state->request->extra_len == 0) {
-		async_request_sent(private_data, True);
+	subreq = wb_simple_trans_send(state, winbind_event_context(), NULL,
+				      state->child->sock, state->request);
+	if (tevent_req_nomem(subreq, req)) {
 		return;
 	}
+	tevent_req_set_callback(subreq, wb_child_request_done, req);
 
-	setup_async_write(&state->child->event, state->request->extra_data.data,
-			  state->request->extra_len,
-			  async_request_sent, state);
-}
-
-/****************************************************************
- Handler triggered if the child winbindd doesn't respond within
- a given timeout.
-****************************************************************/
-
-static void async_request_timeout_handler(struct event_context *ctx,
-					struct timed_event *te,
-					struct timeval now,
-					void *private_data)
-{
-	struct winbindd_async_request *state =
-		talloc_get_type_abort(private_data, struct winbindd_async_request);
-
-	DEBUG(0,("async_request_timeout_handler: child pid %u is not responding. "
-		"Closing connection to it.\n",
-		(unsigned int)state->child_pid ));
-
-	/* Deal with the reply - set to error. */
-	async_reply_recv(private_data, False);
+	if (!tevent_req_set_endtime(req, state->ev,
+				    timeval_current_ofs(300, 0))) {
+		tevent_req_nomem(NULL, req);
+                return;
+        }
 }
 
-/**************************************************************
- Common function called on both async send and recv fail.
- Cleans up the child and schedules the next request.
-**************************************************************/
-
-static void async_request_fail(struct winbindd_async_request *state)
+static void wb_child_request_done(struct tevent_req *subreq)
 {
-	DLIST_REMOVE(state->child->requests, state);
-
-	TALLOC_FREE(state->reply_timeout_event);
-
-	/* If child exists and is not already reaped,
-	   send kill signal to child. */
-
-	if ((state->child->pid != (pid_t)0) &&
-			(state->child->pid != (pid_t)-1) &&
-			(state->child->pid == state->child_pid)) {
-		kill(state->child_pid, SIGTERM);
-
-		/* 
-		 * Close the socket to the child.
-		 */
-		winbind_child_died(state->child_pid);
+	struct tevent_req *req = tevent_req_callback_data(
+		subreq, struct tevent_req);
+	struct wb_child_request_state *state = tevent_req_data(
+		req, struct wb_child_request_state);
+	int ret, err;
+
+	ret = wb_simple_trans_recv(subreq, state, &state->response, &err);
+	TALLOC_FREE(subreq);
+	if (ret == -1) {
+		tevent_req_error(req, err);
+		return;
 	}
-
-	state->response->length = sizeof(struct winbindd_response);
-	state->response->result = WINBINDD_ERROR;
-	state->continuation(state->private_data, False);
+	tevent_req_done(req);
 }
 
-static void async_request_sent(void *private_data_data, bool success)
+int wb_child_request_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
+			  struct winbindd_response **presponse, int *err)
 {
-	struct winbindd_async_request *state =
-		talloc_get_type_abort(private_data_data, struct winbindd_async_request);
+	struct wb_child_request_state *state = tevent_req_data(
+		req, struct wb_child_request_state);
 
-	if (!success) {
-		DEBUG(5, ("Could not send async request to child pid %u\n",
-			(unsigned int)state->child_pid ));
-		async_request_fail(state);
-		return;
+	if (tevent_req_is_unix_error(req, err)) {
+		return -1;
 	}
-
-	/* Request successfully sent to the child, setup the wait for reply */
-
-	setup_async_read(&state->child->event,
-			 &state->response->result,
-			 sizeof(state->response->result),
-			 async_reply_recv, state);
-
-	/* 
-	 * Set up a timeout of 300 seconds for the response.
-	 * If we don't get it close the child socket and
-	 * report failure.
-	 */
-
-	state->reply_timeout_event = event_add_timed(winbind_event_context(),
-							NULL,
-							timeval_current_ofs(300,0),
-							async_request_timeout_handler,
-							state);
-	if (!state->reply_timeout_event) {
-		smb_panic("async_request_sent: failed to add timeout handler.\n");
+	if (state->response->result != WINBINDD_OK) {
+		*err = EIO; /* EIO doesn't fit, but what would be better? */
+		return -1;
 	}
+	*presponse = talloc_move(mem_ctx, &state->response);
+	return 0;
 }
 
-static void async_reply_recv(void *private_data, bool success)
-{
-	struct winbindd_async_request *state =
-		talloc_get_type_abort(private_data, struct winbindd_async_request);
-	struct winbindd_child *child = state->child;
-
-	TALLOC_FREE(state->reply_timeout_event);
-
-	state->response->length = sizeof(struct winbindd_response);
-
-	if (!success) {
-		DEBUG(5, ("Could not receive async reply from child pid %u\n",
-			(unsigned int)state->child_pid ));
-
-		cache_cleanup_response(state->child_pid);
-		async_request_fail(state);
-		return;
-	}
-
-	SMB_ASSERT(cache_retrieve_response(state->child_pid,
-					   state->response));
-
-	cache_cleanup_response(state->child_pid);
-
-	DLIST_REMOVE(child->requests, state);
-
-	schedule_async_request(child);
+/*
+ * Machinery for async requests sent to children. You set up a
+ * winbindd_request, select a child to query, and issue a async_request
+ * call. When the request is completed, the callback function you specified is
+ * called back with the private pointer you gave to async_request.
+ */
 
-	state->continuation(state->private_data, True);
-}
+struct winbindd_async_request {
+	struct winbindd_async_request *next, *prev;
+	TALLOC_CTX *mem_ctx;
+	struct winbindd_child *child;
+	struct winbindd_response *response;
+	void (*continuation)(void *private_data, bool success);
+	struct timed_event *reply_timeout_event;
+	pid_t child_pid; /* pid of the child we're waiting on. Used to detect
+			    a restart of the child (child->pid != child_pid). */
+	void *private_data;
+};
 
 static bool fork_domain_child(struct winbindd_child *child);
+static void async_request_done(struct tevent_req *req);
 
-static void schedule_async_request(struct winbindd_child *child)
+void async_request(TALLOC_CTX *mem_ctx, struct winbindd_child *child,
+		   struct winbindd_request *request,
+		   struct winbindd_response *response,
+		   void (*continuation)(void *private_data, bool success),
+		   void *private_data)
 {
-	struct winbindd_async_request *request = child->requests;
+	struct winbindd_async_request *state;
+	struct tevent_req *req;
 
-	if (request == NULL) {
-		return;
-	}
+	DEBUG(10, ("Sending request to child pid %d (domain=%s)\n",
+		   (int)child->pid,
+		   (child->domain != NULL) ? child->domain->name : "''"));
 
-	if (child->event.flags != 0) {
-		return;		/* Busy */
+	state = talloc(mem_ctx, struct winbindd_async_request);
+	if (state == NULL) {
+		DEBUG(0, ("talloc failed\n"));
+		continuation(private_data, False);
+		return;
 	}
 
-	/*
-	 * This may be a reschedule, so we might
-	 * have an existing timeout event pending on
-	 * the first entry in the child->requests list
-	 * (we only send one request at a time).
-	 * Ensure we free it before we reschedule.
-	 * Bug #5814, from hargagan <shargagan@novell.com>.
-	 * JRA.
-	 */
-
-	TALLOC_FREE(request->reply_timeout_event);
+	state->mem_ctx = mem_ctx;
+	state->child = child;
+	state->reply_timeout_event = NULL;
+	state->response = response;
+	state->continuation = continuation;
+	state->private_data = private_data;
 
-	if ((child->pid == 0) && (!fork_domain_child(child))) {
-		/* fork_domain_child failed.
-		   Cancel all outstanding requests */
+	request->pid = child->pid;
 
-		while (request != NULL) {
-			/* request might be free'd in the continuation */
-			struct winbindd_async_request *next = request->next;
+	req = wb_child_request_send(state, winbind_event_context(),
+					   child, request);
+	if (req == NULL) {
+		DEBUG(0, ("wb_child_request_send failed\n"));
+                continuation(private_data, false);
+		return;
+        }
+	tevent_req_set_callback(req, async_request_done, state);
+}
 
-			async_request_fail(request);
-			request = next;
-		}
+static void async_request_done(struct tevent_req *req)
+{
+	struct winbindd_async_request *state = tevent_req_callback_data(
+		req, struct winbindd_async_request);
+	struct winbindd_response *response;
+	int ret, err;
+
+	ret = wb_child_request_recv(req, state, &response, &err);
+	TALLOC_FREE(req);
+	if (ret == -1) {
+		DEBUG(2, ("wb_child_request_recv failed: %s\n",
+			  strerror(err)));
+		state->continuation(state->private_data, false);
 		return;
 	}
-
-	/* Now we know who we're sending to - remember the pid. */
-	request->child_pid = child->pid;
-
-	setup_async_write(&child->event, request->request,
-			  sizeof(*request->request),
-			  async_main_request_sent, request);
-
-	return;
+	*state->response = *response;
+	state->continuation(state->private_data, true);
 }
 
 struct domain_request_state {
@@ -477,6 +412,8 @@ void setup_child(struct winbindd_child *child,
 
 	child->domain = NULL;
 	child->table = table;
+	child->queue = tevent_queue_create(NULL, "winbind_child");
+	SMB_ASSERT(child->queue != NULL);
 }
 
 struct winbindd_child *children = NULL;
@@ -500,24 +437,9 @@ void winbind_child_died(pid_t pid)
 
 	DLIST_REMOVE(children, child);
 
-	remove_fd_event(&child->event);
-	close(child->event.fd);
-	child->event.fd = 0;
-	child->event.flags = 0;
+	close(child->sock);
+	child->sock = -1;
 	child->pid = 0;
-
-	if (child->requests) {
-		/*
-		 * schedule_async_request() will also
-		 * clear this event but the call is
-		 * idempotent so it doesn't hurt to
-		 * cover all possible future code
-		 * paths. JRA.
-		 */
-		TALLOC_FREE(child->requests->reply_timeout_event);
-	}
-
-	schedule_async_request(child);
 }
 
 /* Ensure any negative cache entries with the netbios or realm names are removed. */
@@ -1176,11 +1098,6 @@ bool winbindd_reinit_after_fork(const char *logfilename)
 
 	/* Destroy all possible events in child list. */
 	for (cl = children; cl != NULL; cl = cl->next) {
-		struct winbindd_async_request *request;
-
-		for (request = cl->requests; request; request = request->next) {
-			TALLOC_FREE(request->reply_timeout_event);
-		}
 		TALLOC_FREE(cl->lockout_policy_event);
 		TALLOC_FREE(cl->machine_password_change_event);
 
@@ -1247,9 +1164,7 @@ static bool fork_domain_child(struct winbindd_child *child)
 		close(fdpair[0]);
 		child->next = child->prev = NULL;
 		DLIST_ADD(children, child);
-		child->event.fd = fdpair[1];
-		child->event.flags = 0;
-		add_fd_event(&child->event);
+		child->sock = fdpair[1];
 		return True;
 	}
 
@@ -1358,6 +1273,8 @@ static bool fork_domain_child(struct winbindd_child *child)
 		struct timeval *tp;
 		struct timeval now;
 		TALLOC_CTX *frame = talloc_stackframe();
+		struct iovec iov[2];
+		int iov_count;
 
 		if (run_events(winbind_event_context(), 0, NULL, NULL)) {
 			TALLOC_FREE(frame);
@@ -1428,18 +1345,27 @@ static bool fork_domain_child(struct winbindd_child *child)
 		state.request->null_term = '\0';
 		child_process_request(child, &state);
 
+		DEBUG(4, ("Finished processing child request %d\n",
+			  (int)state.request->cmd));
+
 		SAFE_FREE(state.request->extra_data.data);
 
-		cache_store_response(sys_getpid(), &state.response);
+		iov[0].iov_base = (void *)&state.response;
+		iov[0].iov_len = sizeof(struct winbindd_response);
+		iov_count = 1;
+
+		if (state.response.length > sizeof(struct winbindd_response)) {
+			iov[1].iov_base =
+				(void *)state.response.extra_data.data;
+			iov[1].iov_len = state.response.length-iov[0].iov_len;
+			iov_count = 2;
+		}
 
-		/* We just send the result code back, the result
-		 * structure needs to be fetched via the
-		 * winbindd_cache. Hmm. That needs fixing... */
+		DEBUG(10, ("Writing %d bytes to parent\n",
+			   (int)state.response.length));
 
-		if (write_data(state.sock,
-			       (const char *)&state.response.result,
-			       sizeof(state.response.result)) !=
-		    sizeof(state.response.result)) {
+		if (write_data_iov(state.sock, iov, iov_count) !=
+		    state.response.length) {
 			DEBUG(0, ("Could not write result\n"));
 			exit(1);
 		}
diff --git a/source3/winbindd/winbindd_ndr.c b/source3/winbindd/winbindd_ndr.c
index 3e3ca3225c..80a071103a 100644
--- a/source3/winbindd/winbindd_ndr.c
+++ b/source3/winbindd/winbindd_ndr.c
@@ -43,7 +43,6 @@ void ndr_print_winbindd_child(struct ndr_print *ndr,
 	ndr_print_string(ndr, "logfilename", r->logfilename);
 	/* struct fd_event event; */
 	ndr_print_ptr(ndr, "lockout_policy_event", r->lockout_policy_event);
-	ndr_print_ptr(ndr, "requests", r->requests);
 	ndr_print_ptr(ndr, "table", r->table);
 	ndr->depth--;
 }
diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h
index 384395f896..55c0af3148 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -276,6 +276,13 @@ void setup_domain_child(struct winbindd_domain *domain,
 
 /* The following definitions come from winbindd/winbindd_dual.c  */
 
+struct tevent_req *wb_child_request_send(TALLOC_CTX *mem_ctx,
+					 struct tevent_context *ev,
+					 struct winbindd_child *child,
+					 struct winbindd_request *request);
+int wb_child_request_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
+			  struct winbindd_response **presponse, int *err);
+
 void async_request(TALLOC_CTX *mem_ctx, struct winbindd_child *child,
 		   struct winbindd_request *request,
 		   struct winbindd_response *response,
diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index 41f9d7de5c..bdd73e2dec 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -373,8 +373,6 @@ static void trustdom_recv(void *private_data, bool success)
 			p += 1;
 	}
 
-	SAFE_FREE(response->extra_data.data);
-
 	/*
 	   Cases to consider when scanning trusts:
 	   (a) we are calling from a child domain (primary && !forest_root)