As near as I can tell, Win32 will now correspond to Unix in terms of the

  availability and lifetime of the hproc process handle.


git-svn-id: http://svn.apache.org/repos/asf/apr/apr/trunk@64388 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 25 insertions, 25 deletions

diff --git a/CHANGES b/CHANGES
index 1db6e9780..8f3a5e897 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,9 @@
 Changes with APR 0.9.2
 
+  *) Alter Win32's handling of the apr_proc_t hproc member, so that we
+     close that system handle wherever an apr function would invoke the
+     final waitpid() against a zombie process on Unix.  [William Rowe]
+
   *) APR_MAX_SECONDS_TO_LINGER
 and APR_FNM_* #defines replace their
      old undecorated names (missing APR_ prefix).  The old names will
      disappear with APR 1.0.0.
diff --git a/memory/unix/apr_pools.c b/memory/unix/apr_pools.c
index 25ddbd9ef..0338233e2 100644
--- a/memory/unix/apr_pools.c
+++ b/memory/unix/apr_pools.c
@@ -2066,6 +2066,7 @@ static void free_proc_chain(struct process_chain *procs)
 #endif /* !defined(NEED_WAITPID) */
 
     for (pc = procs; pc; pc = pc->next) {
+#ifndef WIN32
         if ((pc->kill_how == APR_KILL_AFTER_TIMEOUT)
             || (pc->kill_how == APR_KILL_ONLY_ONCE)) {
             /*
@@ -2074,14 +2075,15 @@ static void free_proc_chain(struct process_chain *procs)
              * similar to a SIGKILL, so always give the process a timeout
              * under Windows before killing it.
              */
-#ifdef WIN32
-            need_timeout = 1;
-#else /* !defined(WIN32) */
             if (apr_proc_kill(pc->proc, SIGTERM) == APR_SUCCESS)
                 need_timeout = 1;
-#endif /* !defined(WIN32) */
         }
         else if (pc->kill_how == APR_KILL_ALWAYS) {
+#else /* WIN32 knows only one fast, clean method of killing processes today */
+        if (pc->kill_how != APR_KILL_NEVER) {
+            need_timeout = 1;
+            pc->kill_how = APR_KILL_ALWAYS;
+#endif
             apr_proc_kill(pc->proc, SIGKILL);
         }
     }
@@ -2130,23 +2132,6 @@ static void free_proc_chain(struct process_chain *procs)
         if (pc->kill_how != APR_KILL_NEVER)
             (void)apr_proc_wait(pc->proc, NULL, NULL, APR_WAIT);
     }
-
-#ifdef WIN32
-    /*
-     * XXX: Do we need an APR function to clean-up a proc_t?
-     * Well ... yeah ... but we can't since it's scope is ill defined.
-     * We can't dismiss the handle until the apr_proc_wait above is
-     * finished with the proc_t.
-     */
-    {
-        for (pc = procs; pc; pc = pc->next) {
-            if (pc->proc->hproc) {
-                CloseHandle(pc->proc->hproc);
-                pc->proc->hproc = NULL;
-            }
-        }
-    }
-#endif /* defined(WIN32) */
 }
 
 
diff --git a/threadproc/win32/proc.c b/threadproc/win32/proc.c
index 9f13d6e7a..ed52f314b 100644
--- a/threadproc/win32/proc.c
+++ b/threadproc/win32/proc.c
@@ -699,7 +699,10 @@ APR_DECLARE(apr_status_t) apr_proc_wait_all_procs(apr_proc_t *proc,
                                                   apr_pool_t *p)
 {
     /* Unix does apr_proc_wait(proc(-1), exitcode, exitwhy, waithow)
-     * but Win32's apr_proc_wait won't work that way.
+     * but Win32's apr_proc_wait won't work that way.  We can either
+     * register all APR created processes in some sort of AsyncWait
+     * thread, or simply walk from the global process pool for all 
+     * apr_pool_note_subprocess()es registered with APR.
      */
     return APR_ENOTIMPL;
 }
@@ -735,6 +738,8 @@ APR_DECLARE(apr_status_t) apr_proc_wait(apr_proc_t *proc,
                 *exitcode = stat;
             if (exitwhy)
                 *exitwhy = why_from_exit_code(stat);
+            CloseHandle(proc->hproc);
+            proc->hproc = NULL;
             return APR_CHILD_DONE;
         }
     }
diff --git a/threadproc/win32/signals.c b/threadproc/win32/signals.c
index b10ccc43d..e83e009fb 100644
--- a/threadproc/win32/signals.c
+++ b/threadproc/win32/signals.c
@@ -65,15 +65,20 @@
 #include <sys/wait.h>
 #endif
 
-/* Windows only really support killing process, but that will do for now. */
+/* Windows only really support killing process, but that will do for now. 
+ *
+ * ### Actually, closing the input handle to the proc should also do fine 
+ * for most console apps.  This definately needs improvement...
+ */
 APR_DECLARE(apr_status_t) apr_proc_kill(apr_proc_t *proc, int signal)
 {
     if (proc->hproc != NULL) {
         if (TerminateProcess(proc->hproc, signal) == 0) {
             return apr_get_os_error();
         }
-        CloseHandle(proc->hproc);
-        proc->hproc = NULL;
+        /* On unix, SIGKILL leaves a apr_proc_wait()able pid lying around, 
+         * so we will leave hproc alone until the app calls apr_proc_wait().
+         */
         return APR_SUCCESS;
     }
     return APR_EPROC_UNKNOWN;
@@ -82,6 +87,7 @@ APR_DECLARE(apr_status_t) apr_proc_kill(apr_proc_t *proc, int signal)
 void apr_signal_init(apr_pool_t *pglobal)
 {
 }
+
 const char *apr_signal_description_get(int signum)
 {
     return "unknown signal (not supported)";