apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@attglobal.net>
Subject Re: [PATCH] Re: cvs commit: apr/threadproc/unix proc.c
Date Tue, 16 Oct 2001 18:12:58 GMT
Ryan Bloom <rbb@covalent.net> writes:

> I would also get rid of the native exit code. 

This is a real pain.  Unfortunately, I only "felt" it and didn't have
any concrete examples until I got mostly done with a new patch :)

Consider the logic in Apache to see if a core dump occurred.  This
isn't a portable concept.  It seems best for APR not to get in-between
the OS and the app.  Otherwise, we may find there are other bits in
the OS status which are of critical importance.  All of this adds more
clumsiness to the APR interface if we try to APR-ize all the info.

I'm happy with the APR-ized notion of how-did-the-process-exit and
what-is-the-signal-or-exit-code but throwing away the native status is
a real problem.

And while at first I was fine with conveying the two pieces of
information in the parameter list to apr_proc_wait() and
apr_proc_wait_all_procs(), if the native status is provided the
parameter list gets a little more unwieldy.

So I have a new patch which I think does what you want (give or take
the names of identifiers), but it does not completely restore the info
we used to be able to get from apr_proc_wait().

Comments from anyone about what to change are most welcome.  Simply
adding the native status to apr_proc_wait*() parmlist is okay with me,
but like I say it seems unwieldy.

missing from patch: 
doc in header files, other mpms, apr/threadproc/foo, where foo !=
unix, testing

Index: include/mpm_common.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/mpm_common.h,v
retrieving revision 1.28
diff -u -r1.28 mpm_common.h
--- include/mpm_common.h	2001/08/14 12:30:49	1.28
+++ include/mpm_common.h	2001/10/16 17:57:09
@@ -125,7 +125,8 @@
  * @param p The pool to allocate out of
  */
 #ifdef AP_MPM_WANT_WAIT_OR_TIMEOUT
-void ap_wait_or_timeout(apr_wait_t *status, apr_proc_t *ret, apr_pool_t *p);
+void ap_wait_or_timeout(apr_exit_how_e *exithow, int *exitcode_or_signal,
+                        apr_proc_t *ret, apr_pool_t *p);
 #endif
 
 /**
@@ -135,7 +136,8 @@
  * @param status The status returned from ap_wait_or_timeout
  */
 #ifdef AP_MPM_WANT_PROCESS_CHILD_STATUS
-void ap_process_child_status(apr_proc_t *pid, apr_wait_t status);
+void ap_process_child_status(apr_proc_t *pid, apr_exit_how_e exithow, 
+                             int exitcode_or_signal);
 #endif
 
 #if defined(TCP_NODELAY) && !defined(MPE) && !defined(TPF)
Index: server/mpm_common.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/mpm_common.c,v
retrieving revision 1.69
diff -u -r1.69 mpm_common.c
--- server/mpm_common.c	2001/09/21 14:29:33	1.69
+++ server/mpm_common.c	2001/10/16 17:57:18
@@ -124,7 +124,7 @@
                 continue;
 
             proc.pid = pid;
-            waitret = apr_proc_wait(&proc, NULL, APR_NOWAIT);
+            waitret = apr_proc_wait(&proc, NULL, NULL, APR_NOWAIT);
             if (waitret != APR_CHILD_NOTDONE) {
                 MPM_NOTE_CHILD_KILLED(i);
                 continue;
@@ -196,7 +196,8 @@
 #endif
 static int wait_or_timeout_counter;
 
-void ap_wait_or_timeout(apr_wait_t *status, apr_proc_t *ret, apr_pool_t *p)
+void ap_wait_or_timeout(apr_exit_how_e *exithow, int *exitcode_or_signal, 
+                        apr_proc_t *ret, apr_pool_t *p)
 {
     apr_status_t rv;
 
@@ -204,7 +205,7 @@
     if (wait_or_timeout_counter == INTERVAL_OF_WRITABLE_PROBES) {
         wait_or_timeout_counter = 0;
     }
-    rv = apr_proc_wait_all_procs(ret, status, APR_NOWAIT, p);
+    rv = apr_proc_wait_all_procs(ret, exithow, exitcode_or_signal, APR_NOWAIT, p);
     if (APR_STATUS_IS_EINTR(rv)) {
         ret->pid = -1;
         return;
@@ -213,6 +214,7 @@
         return;
     }
 #ifdef NEED_WAITPID
+    /* wtf... ancient cruft? */
     if ((ret = reap_children(status)) > 0) {
         return;
     }
@@ -224,16 +226,16 @@
 #endif /* AP_MPM_WANT_WAIT_OR_TIMEOUT */
 
 #ifdef AP_MPM_WANT_PROCESS_CHILD_STATUS
-void ap_process_child_status(apr_proc_t *pid, apr_wait_t status)
+void ap_process_child_status(apr_proc_t *pid, apr_exit_how_e exithow, 
+                             int exitcode_or_signal)
 {
-    int signum = WTERMSIG(status);
-    const char *sigdesc = apr_signal_get_description(signum);
+    const char *sigdesc;
 
     /* Child died... if it died due to a fatal error,
         * we should simply bail out.
         */
-    if ((WIFEXITED(status)) &&
-        WEXITSTATUS(status) == APEXIT_CHILDFATAL) {
+    if (exithow == APR_CHILD_NORMAL_EXIT &&
+        exitcode_or_signal == APEXIT_CHILDFATAL) {
         ap_log_error(APLOG_MARK, APLOG_ALERT|APLOG_NOERRNO, 0, ap_server_conf,
                         "Child %ld returned a Fatal error..." APR_EOL_STR
                         "Apache is exiting!",
@@ -241,22 +243,23 @@
         exit(APEXIT_CHILDFATAL);
     }
 
-    if (WIFSIGNALED(status)) {
-        switch (signum) {
+    if (exithow == APR_CHILD_SIGNAL_EXIT) {
+        switch (exitcode_or_signal) {
         case SIGTERM:
         case SIGHUP:
         case AP_SIG_GRACEFUL:
         case SIGKILL:
             break;
         default:
-#ifdef WCOREDUMP
+            sigdesc = apr_signal_get_description(exitcode_or_signal);
+#ifdef WCOREDUMP_SHOWSTOPPER
             if (WCOREDUMP(status)) {
                 ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_NOTICE,
                              0, ap_server_conf,
                              "child pid %ld exit signal %s (%d), "
                              "possible coredump in %s",
-                             (long)pid->pid, sigdesc, signum,
-                             ap_coredump_dir);
+                             (long)pid->pid, sigdesc,
+                             exitcode_or_signal, ap_coredump_dir);
             }
             else
 #endif
@@ -264,7 +267,7 @@
                 ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_NOTICE,
                              0, ap_server_conf,
                              "child pid %ld exit signal %s (%d)",
-                             (long)pid->pid, sigdesc, signum);
+                             (long)pid->pid, sigdesc, exitcode_or_signal);
             }
         }
     }
Index: server/mpm/prefork/prefork.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/mpm/prefork/prefork.c,v
retrieving revision 1.203
diff -u -r1.203 prefork.c
--- server/mpm/prefork/prefork.c	2001/10/11 16:28:23	1.203
+++ server/mpm/prefork/prefork.c	2001/10/16 17:57:25
@@ -1175,18 +1171,19 @@
 
     while (!restart_pending && !shutdown_pending) {
 	int child_slot;
-	apr_wait_t status;
+        apr_exit_how_e exithow;
+        int exitcode_or_signal;
         /* this is a memory leak, but I'll fix it later. */
 	apr_proc_t pid;
 
-        ap_wait_or_timeout(&status, &pid, pconf);
+        ap_wait_or_timeout(&exithow, &exitcode_or_signal, &pid, pconf);
 
 	/* 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
 	 * extra child
 	 */
 	if (pid.pid != -1) {
-	    ap_process_child_status(&pid, status);
+	    ap_process_child_status(&pid, exithow, exitcode_or_signal);
 	    /* non-fatal death... note that it's gone in the scoreboard. */
 	    ap_sync_scoreboard_image();
 	    child_slot = find_child_by_pid(&pid);
@@ -1203,7 +1200,7 @@
 		}
 #if APR_HAS_OTHER_CHILD
 	    }
-	    else if (apr_proc_other_child_read(&pid, status) == 0) {
+	    else if (apr_proc_other_child_read(&pid, -1 /* status */) == 0) {
 		/* handled */
 #endif
 	    }
Index: srclib/apr/include/apr_thread_proc.h
===================================================================
RCS file: /home/cvs/apr/include/apr_thread_proc.h,v
retrieving revision 1.75
diff -u -r1.75 apr_thread_proc.h
--- srclib/apr/include/apr_thread_proc.h	2001/09/20 08:59:30	1.75
+++ srclib/apr/include/apr_thread_proc.h	2001/10/16 17:57:48
@@ -81,6 +81,7 @@
 
 typedef enum {APR_SHELLCMD, APR_PROGRAM} apr_cmdtype_e;
 typedef enum {APR_WAIT, APR_NOWAIT} apr_wait_how_e;
+typedef enum {APR_CHILD_NORMAL_EXIT, APR_CHILD_SIGNAL_EXIT} apr_exit_how_e;
 
 #define APR_NO_PIPE          0
 #define APR_FULL_BLOCK       1
@@ -481,11 +482,18 @@
 /**
  * Wait for a child process to die
  * @param proc The process handle that corresponds to the desired child process 
- * @param exitcode The returned exit status of the child, if a child process
- *                 dies. On platforms that don't support obtaining this
- *                 information, the exitcode parameter will be returned as
- *                 APR_ENOTIMPL. This parameter may be NULL if the exit code
- *                 is not needed.
+ * @parameter exithow How the child exited.  One of:
+ * <PRE>
+ *            APR_CHILD_NORMAL_EXIT  -- child exited normally
+ *            APR_CHILD_SIGNAL_EXIT  -- child exited due to an uncaught signal
+ * </PRE>
+ * This parameter may be NULL if the reason for exiting is not needed.
+ * @param exitcode_or_signal The returned exit status of the child, if a child 
+ *                 process died normally, or the signal number if the child 
+ *                 process died due to an uncaught signal.  On platforms that 
+ *                 don't support obtaining this information, the exitcode 
+ *                 parameter will be returned as APR_ENOTIMPL. This parameter 
+ *                 may be NULL if the exit code is not needed.
  * @param waithow How should we wait.  One of:
  * <PRE>
  *            APR_WAIT   -- block until the child process dies.
@@ -499,7 +507,8 @@
  * </PRE>
  */
 APR_DECLARE(apr_status_t) apr_proc_wait(apr_proc_t *proc, 
-                                        apr_wait_t *exitcode,
+                                        apr_exit_how_e *exithow,
+                                        int *exitcode_or_signal,
                                         apr_wait_how_e waithow);
 
 /**
@@ -507,9 +516,6 @@
  * about that child.
  * @param proc Pointer to NULL on entry, will be filled out with child's 
  *             information 
- * @param status The returned exit status of the child, if a child process dies
- *               On platforms that don't support obtaining this information, 
- *               the status parameter will be returned as APR_ENOTIMPL.
  * @param waithow How should we wait.  One of:
  * <PRE>
  *            APR_WAIT   -- block until the child process dies.
@@ -519,9 +525,10 @@
  * @param p Pool to allocate child information out of.
  */
 APR_DECLARE(apr_status_t) apr_proc_wait_all_procs(apr_proc_t *proc,
-                                             apr_wait_t *status,
-                                             apr_wait_how_e waithow,
-                                             apr_pool_t *p);
+                                                  apr_exit_how_e *exithow,
+                                                  int *exitcode_or_signal,
+                                                  apr_wait_how_e waithow,
+                                                  apr_pool_t *p);
 
 /**
  * Detach the process from the controlling terminal.
Index: srclib/apr/memory/unix/apr_pools.c
===================================================================
RCS file: /home/cvs/apr/memory/unix/apr_pools.c,v
retrieving revision 1.113
diff -u -r1.113 apr_pools.c
--- srclib/apr/memory/unix/apr_pools.c	2001/09/28 15:22:35	1.113
+++ srclib/apr/memory/unix/apr_pools.c	2001/10/16 17:57:57
@@ -1505,7 +1505,7 @@
 #ifndef NEED_WAITPID
     /* Pick up all defunct processes */
     for (p = procs; p; p = p->next) {
-        if (apr_proc_wait(p->pid, NULL, APR_NOWAIT) != APR_CHILD_NOTDONE) {
+        if (apr_proc_wait(p->pid, NULL, NULL, APR_NOWAIT) != APR_CHILD_NOTDONE) {
             p->kill_how = kill_never;
         }
     }
@@ -1550,7 +1550,7 @@
     /* Now wait for all the signaled processes to die */
     for (p = procs; p; p = p->next) {
 	if (p->kill_how != kill_never) {
-	    (void) apr_proc_wait(p->pid, NULL, APR_WAIT);
+	    (void) apr_proc_wait(p->pid, NULL, NULL, APR_WAIT);
 	}
     }
 #ifdef WIN32
Index: srclib/apr/threadproc/unix/proc.c
===================================================================
RCS file: /home/cvs/apr/threadproc/unix/proc.c,v
retrieving revision 1.49
diff -u -r1.49 proc.c
--- srclib/apr/threadproc/unix/proc.c	2001/09/21 16:14:50	1.49
+++ srclib/apr/threadproc/unix/proc.c	2001/10/16 17:58:01
@@ -362,37 +362,50 @@
 }
 
 APR_DECLARE(apr_status_t) apr_proc_wait_all_procs(apr_proc_t *proc, 
-                                                  apr_wait_t *status,
+                                                  apr_exit_how_e *exithow,
+                                                  int *exitcode_or_signal,
                                                   apr_wait_how_e waithow, 
                                                   apr_pool_t *p)
 {
     proc->pid = -1;
-    return apr_proc_wait(proc, status, waithow);
+    return apr_proc_wait(proc, exithow, exitcode_or_signal, waithow);
 } 
 
 APR_DECLARE(apr_status_t) apr_proc_wait(apr_proc_t *proc, 
-                                        apr_wait_t *exitcode,
+                                        apr_exit_how_e *exithow,
+                                        int *exitcode_or_signal,
                                         apr_wait_how_e waithow)
 {
     pid_t pstatus;
-    int waitpid_options = WUNTRACED;
-    int exit_int;
+    int waitpid_options = 0;
+    int exit_int, ignored_exitcode_or_signal;
+    apr_exit_how_e ignored_exithow;
 
+    /* exithow and exitcode_or_signal are optional */
+    if (!exithow)
+        exithow = &ignored_exithow;
+    if (!exitcode_or_signal)
+        exitcode_or_signal = &ignored_exitcode_or_signal;
+
     if (waithow != APR_WAIT) {
         waitpid_options |= WNOHANG;
     }
     
     if ((pstatus = waitpid(proc->pid, &exit_int, waitpid_options)) > 0) {
-        if (proc->pid == -1) {
-            proc->pid = pstatus;
-        }
+        proc->pid = pstatus;
         if (WIFEXITED(exit_int)) {
-            if (exitcode != NULL) {
-                *exitcode = WEXITSTATUS(exit_int);
-            }
-            return APR_CHILD_DONE;
+            *exithow = APR_CHILD_NORMAL_EXIT;
+            *exitcode_or_signal = WEXITSTATUS(exit_int);
+        }
+        else if (WIFSIGNALED(exit_int)) {
+            *exithow = APR_CHILD_SIGNAL_EXIT;
+            *exitcode_or_signal = WTERMSIG(exit_int);
+        }
+        else {
+            /* unexpected condition */
+            return APR_EGENERAL;
         }
-        return APR_CHILD_NOTDONE;
+        return APR_CHILD_DONE;
     }
     else if (pstatus == 0) {
         return APR_CHILD_NOTDONE;

-- 
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
       http://www.geocities.com/SiliconValley/Park/9289/
             Born in Roswell... married an alien...

Mime
View raw message