Tidy up comments and formatting in d903973c05

diff --git a/win32/win32.c b/win32/win32.c
index 54159ca..024a2a8 100644 (file)
--- a/win32/win32.c
+++ b/win32/win32.c
@@ -123,6 +123,7 @@ static LRESULT  win32_process_message(HWND hwnd, UINT msg,
 #ifdef USE_ITHREADS
 static void            remove_dead_pseudo_process(long child);
 static long            find_pseudo_pid(int pid);
+static HWND            get_hwnd_delay(pTHX, long child, DWORD tries);
 #endif
 
 START_EXTERN_C
@@ -1271,43 +1272,47 @@ my_kill(int pid, int sig)
     return retval;
 }
 
-
-
 #ifdef USE_ITHREADS
-/*
-    will get a child psuedo process hwnd, with retrying and sleeping
-    success is hwnd != INVALID_HANDLE_VALUE, so NULL HWND can be returned
-    ms is milliseconds to sleep/tries, each try is 1 millisec, fatally
-    errors if child psuedo process doesn't schedule and deliver a HWND in the
-    time period specified, 0 milliseconds causes only Sleep(0) to be used
-    with "no" OS delay being given to the calling thread, 0 msis not recommended
-*/
+/* Get a child pseudo-process HWND, with retrying and delaying/yielding.
+ * The "tries" parameter is the number of retries to make, with a Sleep(1)
+ * (waiting and yielding the time slot) between each try. Specifying 0 causes
+ * only Sleep(0) (no waiting and potentially no yielding) to be used, so is not
+ * recommended
+ * Returns an hwnd != INVALID_HANDLE_VALUE (so be aware that NULL can be
+ * returned) or croaks if the child pseudo-process doesn't schedule and deliver
+ * a HWND in the time period allowed.
+ */
 static HWND
-get_hwnd_delay(pTHX, long child, DWORD ms){
+get_hwnd_delay(pTHX, long child, DWORD tries)
+{
     HWND hwnd = w32_pseudo_child_message_hwnds[child];
-/* pseudo-process has not yet properly initialized if hwnd isn't set */
     if (hwnd != INVALID_HANDLE_VALUE) return hwnd;
-/*fast sleep, on some NT Kernels/systems, a Sleep(0) won't deschedule a
-thread 100% of the time since threads are sticked to a CPU for NUMA
-and caching reasons, and the child thread was stickied to a different CPU
-therefore there is no workload on that CPU, and Sleep(0) returns control
-without yielding the time slot
-https://rt.perl.org/rt3/Ticket/Display.html?id=88840
-*/
+
+    /* Pseudo-process has not yet properly initialized since hwnd isn't set.
+     * Fast sleep: On some NT kernels/systems, a Sleep(0) won't deschedule a
+     * thread 100% of the time since threads are attached to a CPU for NUMA and
+     * caching reasons, and the child thread was attached to a different CPU
+     * therefore there is no workload on that CPU and Sleep(0) returns control
+     * without yielding the time slot.
+     * https://rt.perl.org/rt3/Ticket/Display.html?id=88840
+     */
     Sleep(0);
     win32_async_check(aTHX);
-       hwnd = w32_pseudo_child_message_hwnds[child];
+    hwnd = w32_pseudo_child_message_hwnds[child];
     if (hwnd != INVALID_HANDLE_VALUE) return hwnd;
+
     {
-        int count = 0;
-        while (count++ < ms){ /*ms=0 no Sleep(1),just fail by now*/
-            Sleep(1);
-            win32_async_check(aTHX);
-            hwnd = w32_pseudo_child_message_hwnds[child];
-            if (hwnd != INVALID_HANDLE_VALUE) return hwnd;                        
-        }
+       int count = 0;
+       /* No Sleep(1) if tries==0, just fail instead if we get this far. */
+       while (count++ < tries) {
+           Sleep(1);
+           win32_async_check(aTHX);
+           hwnd = w32_pseudo_child_message_hwnds[child];
+           if (hwnd != INVALID_HANDLE_VALUE) return hwnd;
+       }
     }
-    Perl_croak(aTHX_ "panic: child psuedo process was never scheduled");
+
+    Perl_croak(aTHX_ "panic: child pseudo-process was never scheduled");
 }
 #endif
 
@@ -1323,50 +1328,52 @@ win32_kill(int pid, int sig)
        if (child >= 0) {
            HANDLE hProcess = w32_pseudo_child_handles[child];
            switch (sig) {
-           case 0:
-               /* "Does process exist?" use of kill */
-               return 0;
-
-           case 9: {
-                /* kill -9 style un-graceful exit */
-/*do a wait to make sure child starts and isnt in DLL Loader Lock*/
-            HWND hwnd = get_hwnd_delay(aTHX, child, 5);/*XXX change delay*/
-               if (TerminateThread(hProcess, sig)) {
-                    /* Allow the scheduler to finish cleaning up the other thread.
-                     * Otherwise, if we ExitProcess() before another context switch
-                     * happens we will end up with a process exit code of "sig" instead
-                     * of our own exit status.
-                     * See also: https://rt.cpan.org/Ticket/Display.html?id=66016#txn-908976
-                     */
-                    Sleep(0);
-                   remove_dead_pseudo_process(child);
+               case 0:
+                   /* "Does process exist?" use of kill */
                    return 0;
-               }
-           }
-               break;
 
-           default: {
-                HWND hwnd = get_hwnd_delay(aTHX, child, 5);/*XXX change delay*/
-                /* We fake signals to pseudo-processes using Win32
-                 * message queue.  In Win9X the pids are negative already. */
-                if ((hwnd != NULL && PostMessage(hwnd, WM_USER_KILL, sig, 0)) ||
-                    PostThreadMessage(-pid, WM_USER_KILL, sig, 0))
-                {
-                    /* Don't wait for child process to terminate after we send a SIGTERM
-                     * because the child may be blocked in a system call and never receive
-                     * the signal.
-                     */
-                    if (sig == SIGTERM) {
-                        Sleep(0);
-                        w32_pseudo_child_sigterm[child] = 1;
-                    }
-                    /* It might be us ... */
-                    PERL_ASYNC_CHECK();
-                    return 0;
-                }
-               break;
-            }
-            } /* switch */
+               case 9: {
+                   /* kill -9 style un-graceful exit */
+                   /* Do a wait to make sure child starts and isn't in DLL
+                    * Loader Lock */
+                   HWND hwnd = get_hwnd_delay(aTHX, child, 5);
+                   if (TerminateThread(hProcess, sig)) {
+                       /* Allow the scheduler to finish cleaning up the other
+                        * thread.
+                        * Otherwise, if we ExitProcess() before another context
+                        * switch happens we will end up with a process exit
+                        * code of "sig" instead of our own exit status.
+                        * https://rt.cpan.org/Ticket/Display.html?id=66016#txn-908976
+                        */
+                       Sleep(0);
+                       remove_dead_pseudo_process(child);
+                       return 0;
+                   }
+                   break;
+               }
+
+               default: {
+                   HWND hwnd = get_hwnd_delay(aTHX, child, 5);
+                   /* We fake signals to pseudo-processes using Win32
+                    * message queue. */
+                   if ((hwnd != NULL && PostMessage(hwnd, WM_USER_KILL, sig, 0)) ||
+                       PostThreadMessage(-pid, WM_USER_KILL, sig, 0))
+                   {
+                       /* Don't wait for child process to terminate after we send a
+                        * SIGTERM because the child may be blocked in a system call
+                        * and never receive the signal.
+                        */
+                       if (sig == SIGTERM) {
+                           Sleep(0);
+                           w32_pseudo_child_sigterm[child] = 1;
+                       }
+                       /* It might be us ... */
+                       PERL_ASYNC_CHECK();
+                       return 0;
+                   }
+                   break;
+               }
+           } /* switch */
        }
     }
     else