Convert the winbind parent->child communication to wb_reqtrans
authorVolker Lendecke <vl@samba.org>
Sun, 10 May 2009 08:49:53 +0000 (10:49 +0200)
committerVolker Lendecke <vl@samba.org>
Sun, 14 Jun 2009 09:25:47 +0000 (11:25 +0200)
source3/winbindd/winbindd.h
source3/winbindd/winbindd_async.c
source3/winbindd/winbindd_dual.c
source3/winbindd/winbindd_ndr.c
source3/winbindd/winbindd_proto.h
source3/winbindd/winbindd_util.c

index 314643c4e16ede7c9bf57f12ff5b9039877cc5e6..b294b6a4bda945b13f7321b5f5fcb162842cafab 100644 (file)
@@ -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;
 };
index 9f0c6817d88753ebed0da227c0cd3e7bc8fc52ef..af1b67db9c9d67ab41ac94db67e2277b5ca0d4d6 100644 (file)
@@ -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,
index 9a5098f6616d71b46bc5e4c90ff28931115e59ad..53a5bc47a46c3f06367dcca393457c6043d83077 100644 (file)
@@ -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);
                }
index 3e3ca3225c6d78e053fd9b84e4962637e55f8c2c..80a071103ace67f179298cc797ff626f569526f5 100644 (file)
@@ -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--;
 }
index 384395f89648b06fc8da5fb69a1c304bec1566ed..55c0af3148c5dede0d0ac8434ea2882db7abc7f0 100644 (file)
@@ -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,
index 41f9d7de5c33009b5d622fb4051b4ced6d4defd8..bdd73e2dec9d331bd2736ab8bfcab49cb37d3085 100644 (file)
@@ -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)