The branch, master has been updated
       via  df099e6 s3: Avoid a potential 100% CPU loop in winbindd
       via  aa5abca s3: Make winbindd_reinit_after_fork return NTSTATUS
       via  0757688 s3: In winbind, close parent/child sockets
      from  50883cf s3-tevent: only include ../lib/util/tevent wrappers where 
needed.

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit df099e66240c7670c9f7b7dcccb1c38216bac3ec
Author: Volker Lendecke <v...@samba.org>
Date:   Fri Apr 29 13:00:14 2011 +0200

    s3: Avoid a potential 100% CPU loop in winbindd
    
    In the clustering case if ctdb is unhappy, winbindd_reinit_after_fork fails.
    This can lead to an endless loop depending on the scheduling of the parent 
vs
    child. Parent forks, child is immediately scheduled and exits. Parent gets
    SIGCHLD, parent is then scheduled before it sends the request out to the 
child.
    Parent tries to fork again immediately.
    
    The code before this patch did not really take into account that
    reinit_after_fork can fail. The code now sends the result of
    winbindd_reinit_after_fork to the parent and the parent only considers the
    child alive when it got NT_STATUS_OK.
    
    This was seen in 3.4 winbind. winbind has changed significantly since then, 
so
    it might be possible that this does not happen anymore in exactly this way. 
But
    passing up the status of reinit_after_fork and only consider the child alive
    when that's ok is the correct thing to do anyway.
    
    Autobuild-User: Volker Lendecke <vlen...@samba.org>
    Autobuild-Date: Fri Apr 29 17:58:19 CEST 2011 on sn-devel-104

commit aa5abcaf7e2844e3bd3d8e8fe26488673ad3c00e
Author: Volker Lendecke <v...@samba.org>
Date:   Fri Apr 29 12:53:13 2011 +0200

    s3: Make winbindd_reinit_after_fork return NTSTATUS

commit 0757688eb34ec1a22bf8c28f72416d6684756647
Author: Volker Lendecke <v...@samba.org>
Date:   Thu Apr 28 13:26:57 2011 +0200

    s3: In winbind, close parent/child sockets
    
    This should further reduce fd load in winbind children

-----------------------------------------------------------------------

Summary of changes:
 source3/winbindd/winbindd.c       |    6 +++-
 source3/winbindd/winbindd_cm.c    |    6 +++-
 source3/winbindd/winbindd_dual.c  |   53 ++++++++++++++++++++++++++++++++-----
 source3/winbindd/winbindd_proto.h |    3 +-
 4 files changed, 58 insertions(+), 10 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index 033c436..071b3cc 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -378,6 +378,7 @@ static void winbind_msg_validate_cache(struct 
messaging_context *msg_ctx,
 {
        uint8 ret;
        pid_t child_pid;
+       NTSTATUS status;
 
        DEBUG(10, ("winbindd_msg_validate_cache: got validate-cache "
                   "message.\n"));
@@ -404,7 +405,10 @@ static void winbind_msg_validate_cache(struct 
messaging_context *msg_ctx,
 
        /* child */
 
-       if (!winbindd_reinit_after_fork(NULL)) {
+       status = winbindd_reinit_after_fork(NULL, NULL);
+       if (!NT_STATUS_IS_OK(status)) {
+               DEBUG(1, ("winbindd_reinit_after_fork failed: %s\n",
+                         nt_errstr(status)));
                _exit(0);
        }
 
diff --git a/source3/winbindd/winbindd_cm.c b/source3/winbindd/winbindd_cm.c
index 06d954f..d9fca5f 100644
--- a/source3/winbindd/winbindd_cm.c
+++ b/source3/winbindd/winbindd_cm.c
@@ -189,6 +189,7 @@ static bool fork_child_dc_connect(struct winbindd_domain 
*domain)
        TALLOC_CTX *mem_ctx = NULL;
        pid_t parent_pid = sys_getpid();
        char *lfile = NULL;
+       NTSTATUS status;
 
        if (domain->dc_probe_pid != (pid_t)-1) {
                /*
@@ -233,7 +234,10 @@ static bool fork_child_dc_connect(struct winbindd_domain 
*domain)
                }
        }
 
-       if (!winbindd_reinit_after_fork(lfile)) {
+       status = winbindd_reinit_after_fork(NULL, lfile);
+       if (!NT_STATUS_IS_OK(status)) {
+               DEBUG(1, ("winbindd_reinit_after_fork failed: %s\n",
+                         nt_errstr(status)));
                messaging_send_buf(winbind_messaging_context(),
                                   pid_to_procid(parent_pid),
                                   MSG_WINBIND_FAILED_TO_GO_ONLINE,
diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c
index 04ef3d8..ebafe8f 100644
--- a/source3/winbindd/winbindd_dual.c
+++ b/source3/winbindd/winbindd_dual.c
@@ -1167,7 +1167,8 @@ static void child_msg_dump_event_list(struct 
messaging_context *msg,
        dump_event_list(winbind_event_context());
 }
 
-bool winbindd_reinit_after_fork(const char *logfilename)
+NTSTATUS winbindd_reinit_after_fork(const struct winbindd_child *myself,
+                                   const char *logfilename)
 {
        struct winbindd_domain *domain;
        struct winbindd_child *cl;
@@ -1180,7 +1181,7 @@ bool winbindd_reinit_after_fork(const char *logfilename)
                true);
        if (!NT_STATUS_IS_OK(status)) {
                DEBUG(0,("reinit_after_fork() failed\n"));
-               return false;
+               return status;
        }
 
        close_conns_after_fork();
@@ -1191,10 +1192,10 @@ bool winbindd_reinit_after_fork(const char *logfilename)
        }
 
        if (!winbindd_setup_sig_term_handler(false))
-               return false;
+               return NT_STATUS_NO_MEMORY;
        if (!winbindd_setup_sig_hup_handler(override_logfile ? NULL :
                                            logfilename))
-               return false;
+               return NT_STATUS_NO_MEMORY;
 
        /* Stop zombies in children */
        CatchChild();
@@ -1242,6 +1243,14 @@ bool winbindd_reinit_after_fork(const char *logfilename)
                 * go through the parent.
                 */
                cl->pid = (pid_t)0;
+
+               /*
+                * Close service sockets to all other children
+                */
+               if ((cl != myself) && (cl->sock != -1)) {
+                       close(cl->sock);
+                       cl->sock = -1;
+               }
         }
        /*
         * This is a little tricky, children must not
@@ -1262,7 +1271,7 @@ bool winbindd_reinit_after_fork(const char *logfilename)
        cl = idmap_child();
        cl->pid = (pid_t)0;
 
-       return true;
+       return NT_STATUS_OK;
 }
 
 /*
@@ -1282,6 +1291,8 @@ static bool fork_domain_child(struct winbindd_child 
*child)
        struct winbindd_request request;
        struct winbindd_response response;
        struct winbindd_domain *primary_domain = NULL;
+       NTSTATUS status;
+       ssize_t nwritten;
 
        if (child->domain) {
                DEBUG(10, ("fork_domain_child called for domain '%s'\n",
@@ -1310,7 +1321,25 @@ static bool fork_domain_child(struct winbindd_child 
*child)
 
        if (child->pid != 0) {
                /* Parent */
+               ssize_t nread;
+
                close(fdpair[0]);
+
+               nread = read(fdpair[1], &status, sizeof(status));
+               if (nread != sizeof(status)) {
+                       DEBUG(1, ("fork_domain_child: Could not read child 
status: "
+                                 "nread=%d, error=%s\n", (int)nread,
+                                 strerror(errno)));
+                       close(fdpair[1]);
+                       return false;
+               }
+               if (!NT_STATUS_IS_OK(status)) {
+                       DEBUG(1, ("fork_domain_child: Child status is %s\n",
+                                 nt_errstr(status)));
+                       close(fdpair[1]);
+                       return false;
+               }
+
                child->next = child->prev = NULL;
                DLIST_ADD(winbindd_children, child);
                child->sock = fdpair[1];
@@ -1325,7 +1354,18 @@ static bool fork_domain_child(struct winbindd_child 
*child)
        state.sock = fdpair[0];
        close(fdpair[1]);
 
-       if (!winbindd_reinit_after_fork(child->logfilename)) {
+       status = winbindd_reinit_after_fork(child, child->logfilename);
+
+       nwritten = write(state.sock, &status, sizeof(status));
+       if (nwritten != sizeof(status)) {
+               DEBUG(1, ("fork_domain_child: Could not write status: "
+                         "nwritten=%d, error=%s\n", (int)nwritten,
+                         strerror(errno)));
+               _exit(0);
+       }
+       if (!NT_STATUS_IS_OK(status)) {
+               DEBUG(1, ("winbindd_reinit_after_fork failed: %s\n",
+                         nt_errstr(status)));
                _exit(0);
        }
 
@@ -1425,7 +1465,6 @@ static bool fork_domain_child(struct winbindd_child 
*child)
                TALLOC_CTX *frame = talloc_stackframe();
                struct iovec iov[2];
                int iov_count;
-               NTSTATUS status;
 
                if (run_events_poll(winbind_event_context(), 0, NULL, 0)) {
                        TALLOC_FREE(frame);
diff --git a/source3/winbindd/winbindd_proto.h 
b/source3/winbindd/winbindd_proto.h
index 5b48a99..d0619f6 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -291,7 +291,8 @@ void winbind_msg_ip_dropped_parent(struct messaging_context 
*msg_ctx,
                                   uint32_t msg_type,
                                   struct server_id server_id,
                                   DATA_BLOB *data);
-bool winbindd_reinit_after_fork(const char *logfilename);
+NTSTATUS winbindd_reinit_after_fork(const struct winbindd_child *myself,
+                                   const char *logfilename);
 struct winbindd_domain *wb_child_domain(void);
 
 /* The following definitions come from winbindd/winbindd_group.c  */


-- 
Samba Shared Repository

Reply via email to