httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dean Gaudet <dgau...@arctic.org>
Subject RFC: register_other_child
Date Sun, 27 Jul 1997 19:12:09 GMT
Ok this hasn't been tested ... and I haven't implemented any reliable logs
with it yet.  I'm just looking for comments on the api.  Skip down to the
http_main.h diff to see the interface.

The one addition that we haven't talked about before is the "writable fd" 
test.  I see the following modes of failure for a logging child: 

- it dies
- it stops reading

We talked about the first, but not the second.  The approach I use here is
to select() on the pipe to see if it's writable.  If it isn't, then I tell
the child's owner. 

Oh yeah, the below removes reap_children() because it is totally bogus. 
It's supposed to be used when NEED_WAITPID is defined, but it uses waitpid
itself.  This means that UTS21 and NEWSOS have been broken for some time. 

Dean

Index: http_main.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_main.c,v
retrieving revision 1.187
diff -u -r1.187 http_main.c
--- http_main.c	1997/07/24 04:35:47	1.187
+++ http_main.c	1997/07/27 19:06:07
@@ -216,6 +216,20 @@
 
 int one_process = 0;
 
+/* used to maintain list of children which aren't part of the scoreboard */
+typedef struct other_child_rec other_child_rec;
+struct other_child_rec {
+    other_child_rec *next;
+    int pid;
+    void (*maintenance)(int, void *, int);
+    void *data;
+    int write_fd;
+};
+static other_child_rec *other_children;
+
+pool *pconf;			/* Pool for config stuff */
+pool *ptrans;			/* Pool for per-transaction stuff */
+
 /* small utility macros to make things easier to read */
 
 #ifdef WIN32
@@ -745,6 +759,105 @@
 #endif /* ndef NO_LINGCLOSE */
 
 /*****************************************************************
+ * dealing with other children
+ */
+
+void register_other_child (int pid,
+    void (*maintenance)(int reason, void *, int status),
+    void *data, int write_fd)
+{
+    other_child_rec *ocr;
+
+    ocr = palloc (pconf, sizeof (*ocr));
+    ocr->pid = pid;
+    ocr->maintenance = maintenance;
+    ocr->data = data;
+    ocr->write_fd = write_fd;
+    ocr->next = other_children;
+    other_children = ocr;
+}
+
+/* note that since this can be called by a maintenance function while we're
+ * scanning the other_children list, all scanners should protect themself
+ * by loading ocr->next before calling any maintenance function.
+ */
+void unregister_other_child (void *data)
+{
+    other_child_rec **pocr, *nocr;
+
+    for (pocr = &other_children; *pocr; pocr = &(*pocr)->next) {
+	if ((*pocr)->data == data) {
+	    nocr = (*pocr)->next;
+	    (*(*pocr)->maintenance)(OC_REASON_UNREGISTER, (*pocr)->data, -1);
+	    *pocr = nocr;
+	    /* XXX: um, well we've just wasted some space in pconf ? */
+	    return;
+	}
+    }
+}
+
+/* test to ensure that the write_fds are all still writable, otherwise
+ * invoke the maintenance functions as appropriate */
+static void probe_writable_fds (void)
+{
+    fd_set writable_fds;
+    int fd_max;
+    other_child_rec *ocr, *nocr;
+    struct timeval tv;
+    int rc;
+
+    if (other_children == NULL) return;
+
+    fd_max = 0;
+    FD_ZERO (&writable_fds);
+    for (ocr = other_children; ocr; ocr = ocr->next) {
+	if (ocr->write_fd == -1) continue;
+	FD_SET (ocr->write_fd, &writable_fds);
+	if (ocr->write_fd > fd_max) {
+	    fd_max = ocr->write_fd;
+	}
+    }
+    if (fd_max == 0) return;
+
+    do {
+	tv.tv_sec = 0;
+	tv.tv_usec = 0;
+	rc = ap_select (fd_max + 1, NULL, &writable_fds, NULL, &tv);
+    } while (rc == -1 && errno == EINTR);
+
+    if (rc == -1) {
+	/* XXX: uhh this could be really bad, we could have a bad file
+	 * descriptor due to a bug in one of the maintenance routines */
+	log_unixerr ("probe_writable_fds", "select", "could not probe
+	    writable fds", server_conf);
+	return;
+    }
+    if (rc == 0) return;
+
+    for (ocr = other_children; ocr; ocr = nocr) {
+	nocr = ocr->next;
+	if (ocr->write_fd == -1) continue;
+	if (FD_ISSET (ocr->write_fd, &writable_fds)) continue;
+	(*ocr->maintenance)(OC_REASON_UNWRITABLE, ocr->data, -1);
+    }
+}
+
+/* possibly reap an other_child, return 0 if yes, -1 if not */
+static int reap_other_child (int pid, int status)
+{
+    other_child_rec *ocr, *nocr;
+
+    for (ocr = other_children; ocr; ocr = nocr) {
+	nocr = ocr->next;
+	if (ocr->pid != pid) continue;
+	ocr->pid = -1;
+	(*ocr->maintenance)(OC_REASON_DEATH, ocr->data, status);
+	return 0;
+    }
+    return -1;
+}
+
+/*****************************************************************
  *
  * Dealing with the scoreboard... a lot of these variables are global
  * only to avoid getting clobbered by the longjmp() that happens when
@@ -1276,102 +1389,117 @@
     return -1;
 }
 
+
+/* reclaim child processes after a SIGHUP ... the scoreboard is going to
+ * be tossed away in this case */
 static void reclaim_child_processes (void)
 {
 #ifndef MULTITHREAD
     int i, status;
     int my_pid = getpid();
-
+    long int waittime = 4096; /* in usecs */
+    struct timeval tv;
+    int waitret, tries;
+    int not_dead_yet;
+    other_child_rec *ocr, *nocr;
+
+    /* totally ignore these signals while doing this */
+    signal (SIGUSR1, SIG_IGN);
+    signal (SIGHUP, SIG_IGN);
     sync_scoreboard_image();
-    for (i = 0; i < max_daemons_limit; ++i) {
-	int pid = scoreboard_image->servers[i].x.pid;
 
-	if (pid != my_pid && pid != 0) { 
-	    int waitret = 0,
-		tries = 1;
-
-	    while (waitret == 0 && tries <= 4) {
-		long int waittime = 4096; /* in usecs */
-		struct timeval tv;
-	    
-		/* don't want to hold up progress any more than 
-		 * necessary, so keep checking to see if the child
-		 * has exited with an exponential backoff.
-		 * Currently set for a maximum wait of a bit over
-		 * four seconds.
-		 */
-		while (((waitret = waitpid(pid, &status, WNOHANG)) == 0) &&
-			 waittime < 3000000) {
-		       tv.tv_sec = waittime / 1000000;
-		       tv.tv_usec = waittime % 1000000;
-		       waittime = waittime * 2;
-		       ap_select(0, NULL, NULL, NULL, &tv);
-		}
-		if (waitret == 0) {
-		    switch (tries) {
-		    case 1:
-			/* perhaps it missed the SIGHUP, lets try again */
-			log_printf(server_conf, "child process %d did not exit, sending another SIGHUP", pid);
-			kill(pid, SIGHUP);
-			break;
-		    case 2:
-			/* ok, now it's being annoying */
-			log_printf(server_conf, "child process %d still did not exit, sending a SIGTERM", pid);
-			kill(pid, SIGTERM);
-			break;
-		    case 3:
-			/* die child scum */
-			log_printf(server_conf, "child process %d still did not exit, sending a SIGKILL", pid);
-			kill(pid, SIGKILL);
-			break;
-		    case 4:
-			/* gave it our best shot, but alas...  If this really 
-			 * is a child we are trying to kill and it really hasn't
-			 * exited, we will likely fail to bind to the port
-			 * after the restart.
-			 */
-			log_printf(server_conf, "could not make child process %d exit, attempting to continue
anyway", pid);
-			break;
-		    }
-		}
-		tries++;
+    tries = 0;
+    for(tries = 0; tries < 4; ++tries) {
+	/* don't want to hold up progress any more than 
+	* necessary, but we need to allow children a few moments to exit.
+	* delay with an exponential backoff.
+	* Currently set for a maximum wait of a bit over
+	* four seconds.
+	*/
+	tv.tv_sec = waittime / 1000000;
+	tv.tv_usec = waittime % 1000000;
+	waittime = waittime * 2;
+	ap_select(0, NULL, NULL, NULL, &tv);
+
+	/* now see who is done */
+	not_dead_yet = 0;
+	for (i = 0; i < max_daemons_limit; ++i) {
+	    int pid = scoreboard_image->servers[i].x.pid;
+
+	    if (pid == my_pid || pid == 0) continue;
+
+	    waitret = waitpid (pid, &status, WNOHANG);
+	    if (waitret == pid || waitret == -1) {
+		scoreboard_image->servers[i].x.pid = 0;
+		continue;
+	    }
+	    ++not_dead_yet;
+	    switch (tries) {
+	    case 1:
+		/* perhaps it missed the SIGHUP, lets try again */
+		log_printf(server_conf, "child process %d did not exit, "
+					"sending another SIGHUP", pid);
+		kill(pid, SIGHUP);
+		break;
+	    case 2:
+		/* ok, now it's being annoying */
+		log_printf(server_conf, "child process %d still did not exit, "
+					"sending a SIGTERM", pid);
+		kill(pid, SIGTERM);
+		break;
+	    case 3:
+		/* die child scum */
+		log_printf(server_conf, "child process %d still did not exit, "
+					"sending a SIGKILL", pid);
+		kill(pid, SIGKILL);
+		break;
+	    case 4:
+		/* gave it our best shot, but alas...  If this really 
+		    * is a child we are trying to kill and it really hasn't
+		    * exited, we will likely fail to bind to the port
+		    * after the restart.
+		    */
+		log_printf(server_conf, "could not make child process %d exit, "
+					"attempting to continue anyway", pid);
+		break;
 	    }
 	}
-    }
-#endif /* ndef MULTITHREAD */
-}
-
-#if defined(BROKEN_WAIT) || defined(NEED_WAITPID)
-/*
-Some systems appear to fail to deliver dead children to wait() at times.
-This sorts them out. In fact, this may have been caused by a race condition
-in wait_or_timeout(). But this routine is still useful for systems with no
-waitpid().
-*/
-int reap_children (void)
-{
-    int status, n;
-    int ret = 0;
-
-    for (n = 0; n < max_daemons_limit; ++n) {
-	if (scoreboard_image->servers[n].status != SERVER_DEAD
-		&& waitpid (scoreboard_image->servers[n].x.pid, &status, WNOHANG)
-		    == -1
-		&& errno == ECHILD) {
-	    sync_scoreboard_image ();
-	    update_child_status (n, SERVER_DEAD, NULL);
-	    ret = 1;
+	for (ocr = other_children; ocr; ocr = nocr) {
+	    nocr = ocr->next;
+	    if (ocr->pid == -1) continue;
+
+	    waitret = waitpid (ocr->pid, &status, WNOHANG);
+	    if (waitret == ocr->pid) {
+		ocr->pid = -1;
+		(*ocr->maintenance)(OC_REASON_DEATH, ocr->data, status);
+	    } else if (waitret == 0) {
+		(*ocr->maintenance)(OC_REASON_RESTART, ocr->data, -1);
+		++not_dead_yet;
+	    } else if (waitret == -1) {
+		/* uh what the heck? they didn't call unregister? */
+		ocr->pid = -1;
+		(*ocr->maintenance)(OC_REASON_LOST, ocr->data, -1);
+	    }
+	}
+	if (!not_dead_yet) {
+	    /* nothing left to wait for */
+	    break;
 	}
     }
-    return ret;
+#endif /* ndef MULTITHREAD */
 }
-#endif
 
 /* Finally, this routine is used by the caretaker process to wait for
  * a while...
  */
 
-static int wait_or_timeout (void)
+/* number of calls to wait_or_timeout between writable probes */
+#ifndef INTERVAL_OF_WRITABLE_PROBES
+#define INTERVAL_OF_WRITABLE_PROBES 10
+#endif
+static int wait_or_timeout_counter;
+
+static int wait_or_timeout (int *status)
 {
 #ifdef WIN32
 #define MAXWAITOBJ MAXIMUM_WAIT_OBJECTS
@@ -1408,21 +1536,20 @@
 
 #else /* WIN32 */
     struct timeval tv;
-#ifndef NEED_WAITPID
     int ret;
 
-    ret = waitpid (-1, NULL, WNOHANG);
+    ++wait_or_timeout_counter;
+    if (wait_or_timeout_counter == INTERVAL_OF_WRITABLE_PROBES) {
+	wait_or_timeout_counter = 0;
+	probe_writable_fds();
+    }
+    ret = waitpid (-1, status, WNOHANG);
     if (ret == -1 && errno == EINTR) {
 	return -1;
     }
     if (ret > 0) {
 	return ret;
     }
-#else
-    if (reap_children ()) {
-	return -1;
-    }
-#endif
     tv.tv_sec = SCOREBOARD_MAINTENANCE_INTERVAL / 1000000;
     tv.tv_usec = SCOREBOARD_MAINTENANCE_INTERVAL % 1000000;
     ap_select(0, NULL, NULL, NULL, &tv);
@@ -1672,12 +1799,8 @@
 
 /*****************************************************************
  * Connection structures and accounting...
- * Should these be global?  Only to this file, at least...
  */
 
-pool *pconf;			/* Pool for config stuff */
-pool *ptrans;			/* Pool for per-transaction stuff */
-
 static server_rec *find_virtual_server (struct in_addr server_ip,
 				unsigned port, server_rec *server)
 {
@@ -2659,7 +2782,8 @@
 
 	while (!restart_pending) {
 	    int child_slot;
-	    int pid = wait_or_timeout ();
+	    int status;
+	    int pid = wait_or_timeout (&status);
 
 	    /* XXX: if it takes longer than 1 second for all our children
 	     * to start up and get into IDLE state then we may spawn an
@@ -2683,6 +2807,8 @@
 			/* don't perform idle maintenance yet */
 			continue;
 		    }
+		} else if (reap_other_child (pid, status) == 0) {
+		    /* handled */
 		} else if (is_graceful) {
 		    /* Great, we've probably just lost a slot in the
 		     * scoreboard.  Somehow we don't know about this
Index: http_main.h
===================================================================
RCS file: /export/home/cvs/apache/src/http_main.h,v
retrieving revision 1.16
diff -u -r1.16 http_main.h
--- http_main.h	1997/07/21 05:53:46	1.16
+++ http_main.h	1997/07/27 19:06:07
@@ -96,3 +96,40 @@
 void time_process_request (int child_num, int status);
 unsigned int set_callback_and_alarm(void (*fn)(int), int x);
 int check_alarm(void);
+
+/*
+ * register an other_child -- a child which the main loop keeps track of
+ * and knows it is different than the rest of the scoreboard.
+ *
+ * pid is the pid of the child.
+ *
+ * maintenance is a function that is invoked with a reason, the data
+ * pointer passed here, and when appropriate a status result from waitpid().
+ *
+ * write_fd is an fd that is probed for writing by select() if it is ever
+ * unwritable, then maintenance is invoked with reason OC_REASON_UNWRITABLE.
+ * This is useful for log pipe children, to know when they've blocked.  To
+ * disable this feature, use -1 for write_fd.
+ */
+API_EXPORT(void) register_other_child (int pid,
+    void (*maintenance)(int reason, void *data, int status), void *data,
+    int write_fd);
+#define OC_REASON_DEATH		0	/* child has died, caller must call
+                                         * unregister still */
+#define OC_REASON_UNWRITABLE	1	/* write_fd is unwritable */
+#define OC_REASON_RESTART	2	/* a restart is occuring, perform
+					 * any necessary cleanup (including
+					 * sending a special signal to child)
+					 */
+#define OC_REASON_UNREGISTER	3	/* unregister has been called, do
+					 * whatever is necessary (including
+					 * kill the child) */
+#define OC_REASON_LOST		4	/* somehow the child exited without
+					 * us knowing ... buggy os? */
+
+/*
+ * unregister an other_child.  Note that the data pointer is used here, and
+ * is assumed to be unique per other_child.  This is because the pid and
+ * write_fd are possibly killed off separately.
+ */
+API_EXPORT(void) unregister_other_child (void *data);



Mime
View raw message