[PATCH] Don't send MSG_WINBIND_ONLINE to idmap_child() in any child process

[Jeremy Allison]

On Tue, Jan 13, 2009 at 05:59:39PM +0800, boyang wrote:
> Hi, everyone:
>      If the trusted domain cannot go online, fork_child_dc_connect()
> will be invoked periodically to fork() one child process, trying to
> connect to DC.  look, fork_child_dc_connect() ---> getdcs() --->
> get_dc_name_via_netlogon() ---> cm_connect_netlogon() --->
> init_dc_connection() ---> cm_open_connection ---> set_domain_online(),
> here send MSG_WINBIND_ONLINE to idmap_child(). It causes
> MSG_WINBIND_ONLINE being sent to idmap_child() periodically, which cause
> idmap child to fork and connect to the primary domain'DC even primary
> domain is already online. :-)  It is very expensive, we must not want to
> do that....
> 
> Please review the patches!

Ok, I've reviewed this very carefully, and I think you're
correct in this. I've broadened your patch slightly to add
code to null out the pid on all children after fork, as no
child should be sending messages to each other, it should
always go through the parent. Can you check this patch (designed
for 3-2-test) out and let me know if it works for you (should
do - it's a simple change to your code).

Once I get confirmation I'll commit to all branhes under
your name, thanks !

Jeremy.

-------------- next part --------------
diff --git a/source/winbindd/winbindd_dual.c b/source/winbindd/winbindd_dual.c
index 9e2e18f..9922f15 100644
--- a/source/winbindd/winbindd_dual.c
+++ b/source/winbindd/winbindd_dual.c
@@ -1101,8 +1101,32 @@ bool winbindd_reinit_after_fork(const char *logfilename)
 			TALLOC_FREE(request->reply_timeout_event);
 		}
 		TALLOC_FREE(cl->lockout_policy_event);
+		/* Children should never be able
+		 * to send each other messages, all
+		 * messages must go through the parent.
+		 */
+		cl->pid = (pid_t)0;
         }
 
+	/*
+	 * This is a little tricky, children must not
+	 * send an MSG_WINBIND_ONLINE message to idmap_child().
+	 * If we are in a child of our primary domain or
+	 * in the process created by fork_child_dc_connect(),
+	 * and the primary domain cannot go online,
+	 * fork_child_dc_connection() sends MSG_WINBIND_ONLINE 
+	 * periodically to idmap_child().
+	 *
+	 * The sequence is, fork_child_dc_connect() ---> getdcs() --->
+	 * get_dc_name_via_netlogon() ---> cm_connect_netlogon()
+	 * ---> init_dc_connection() ---> cm_open_connection --->
+	 * set_domain_online(), sends MSG_WINBIND_ONLINE to
+	 * idmap_child(). Disallow children sending messages
+	 * to each other, all messages must go through the parent.
+	 */
+	cl = idmap_child();
+	cl->pid = (pid_t)0;
+
 	return true;
 }
 
@@ -1194,6 +1218,14 @@ static bool fork_domain_child(struct winbindd_child *child)
 			}
 		}
 	}
+	
+	/*
+	 * If we are in idmap child, make sure that we set the
+	 * check_online_event to bring primary domain online.
+	 */
+	if (child == idmap_child()) {
+		set_domain_online_request(primary_domain);
+	}
 
 	/* We might be in the idmap child...*/
 	if (child->domain && !(child->domain->internal) &&