[PATCH] Win32 apr_proc cleanup

[Mladen Turk]

Hi,

I think that this is one of the showstoppers.

When creating a process the patch allocates an extra struct from the
pool that can be used in pool cleanup to close the process handle but
only if the proccess is not active. It registers the cleanup function to
that pool.

If the process is still active it can be terminated (I've put that
inside #if 0), but I'm not sure if that's what we need, but it can be
handy.

Comments?

MT.

Index: proc.c
===================================================================
RCS file: /home/cvspublic/apr/threadproc/win32/proc.c,v
retrieving revision 1.59
diff -u -r1.59 proc.c
--- proc.c      2001/10/23 17:56:42     1.59
+++ proc.c      2001/11/22 09:13:22
@@ -65,6 +65,50 @@
 #include <string.h>
 #include <process.h>
 
+/** The opaque pool cleanup struct */
+typedef struct win_proc_t win_proc_t;
+
+struct win_proc_t {
+    HANDLE hProcess;
+    DWORD  dwProcessId;
+};
+
+apr_status_t win_proc_cleanup(void *theProc)
+{
+    win_proc_t *proc = theProc;
+    DWORD  exit_code;
+    
+    if (proc->hProcess != INVALID_HANDLE_VALUE && 
+        GetExitCodeProcess(proc->hProcess, &exit_code)) {
+        if (exit_code != STILL_ACTIVE) {
+            CloseHandle(proc->hProcess);
+            proc->hProcess = INVALID_HANDLE_VALUE;
+            OutputDebugString("Closing NonActive Process HANDLE");
+            return APR_SUCCESS;
+        }
+#if 0
+#pragma message("Proccess termination enabled !")
+        /* XXX: Should we terminate the process 
+         * on apr_pool_clean if the process is still active? 
+        */
+        else {
+            if (OpenProcess(PROCESS_ALL_ACCESS, FALSE, proc->dwProcessId)) {
+                if (TerminateProcess(proc->hProcess, 1)) {   
+                    CloseHandle(proc->hProcess);
+                    proc->hProcess = INVALID_HANDLE_VALUE;
+                    return APR_SUCCESS;
+                }
+            }
+            return APR_EACCES;
+        }
+#else
+        else
+            return APR_EBUSY;
+#endif
+    }
+    return APR_NOTFOUND;
+}
+
 /* 
  * some of the ideas expressed herein are based off of Microsoft
  * Knowledge Base article: Q190351
@@ -296,6 +340,7 @@
     char *pEnvBlock;
     PROCESS_INFORMATION pi;
     DWORD dwCreationFlags = 0;
+    win_proc_t *phandle;
 
     new->in = attr->parent_in;
     new->err = attr->parent_err;
@@ -406,9 +451,9 @@
                 pNext = wcschr(pNext, L'\0') + 1;
                 i++;
             }
-           if (!i)
+        if (!i)
                 *(pNext++) = L'\0';
-           *pNext = L'\0';
+        *pNext = L'\0';
         }
         else 
 #endif /* APR_HAS_UNICODE_FS */
@@ -423,9 +468,9 @@
                 pNext = strchr(pNext, '\0') + 1;
                 i++;
             }
-           if (!i)
+        if (!i)
                 *(pNext++) = '\0';
-           *pNext = '\0';
+        *pNext = '\0';
         }
     } 
 
@@ -536,6 +581,13 @@
         apr_file_close(attr->child_err);
     }
     CloseHandle(pi.hThread);
+    
+    /* create the cleanup sruct */
+    phandle = apr_palloc(cont, sizeof(win_proc_t));
+    phandle->hProcess = pi.hProcess;
+    phandle->dwProcessId = pi.dwProcessId;
+    apr_pool_cleanup_register(cont, (void *)phandle, win_proc_cleanup,
+                              apr_pool_cleanup_null);
 
     return APR_SUCCESS;
 }
@@ -560,7 +612,7 @@
             if (exitcode)
                 *exitcode = (apr_wait_t)stat;
             CloseHandle(proc->hproc);
-            proc->hproc = NULL;
+            proc->hproc = INVALID_HANDLE_VALUE;
             return APR_CHILD_DONE;
         }
     }