forkfd: Restore errno on exit from the SIGCHLD handler

I'd never thought about it, but it is a requirement: a signal handler
must leave the global state as it found it (except for those bits that
it intended to change, and those must be done in an async-signal-safe
way). Otherwise, errno could change from one line to the next in the
middle of some code.

[ChangeLog][QtCore][QProcess] On Unix, the QProcess SIGCHLD handler now
restores errno on exit.

Task-number: QTBUG-68472
Change-Id: If025d476890745368955fffd1531e7126f1436d9
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@qt.io>

1 files changed, 45 insertions, 40 deletions

diff --git a/src/3rdparty/forkfd/forkfd.c b/src/3rdparty/forkfd/forkfd.c
index 7f02ee9a22..bef109e401 100644
--- a/src/3rdparty/forkfd/forkfd.c
+++ b/src/3rdparty/forkfd/forkfd.c
@@ -283,6 +283,7 @@ static void notifyAndFreeInfo(Header *header, ProcessInfo *entry,
     freeInfo(header, entry);
 }
 
+static void reapChildProcesses();
 static void sigchld_handler(int signum, siginfo_t *handler_info, void *handler_context)
 {
     /*
@@ -307,19 +308,25 @@ static void sigchld_handler(int signum, siginfo_t *handler_info, void *handler_c
     }
 
     if (ffd_atomic_load(&forkfd_status, FFD_ATOMIC_RELAXED) == 1) {
-        /* is this one of our children? */
-        BigArray *array;
-        siginfo_t info;
-        struct pipe_payload payload;
-        int i;
+        int saved_errno = errno;
+        reapChildProcesses();
+        errno = saved_errno;
+    }
+}
 
-        memset(&info, 0, sizeof info);
-        memset(&payload, 0, sizeof payload);
+static inline void reapChildProcesses()
+{
+    /* is this one of our children? */
+    BigArray *array;
+    siginfo_t info;
+    struct pipe_payload payload;
+    int i;
 
-#ifdef HAVE_WAITID
-        if (!waitid_works)
-            goto search_arrays;
+    memset(&info, 0, sizeof info);
+    memset(&payload, 0, sizeof payload);
 
+#ifdef HAVE_WAITID
+    if (waitid_works) {
         /* be optimistic: try to see if we can get the child that exited */
 search_next_child:
         /* waitid returns -1 ECHILD if there are no further children at all;
@@ -371,12 +378,34 @@ search_next_child:
          * belongs to one of the chained SIGCHLD handlers. However, there might be another
          * child that exited and does belong to us, so we need to check each one individually.
          */
+    }
+#endif
 
-search_arrays:
+    for (i = 0; i < (int)sizeofarray(children.entries); ++i) {
+        int pid = ffd_atomic_load(&children.entries[i].pid, FFD_ATOMIC_ACQUIRE);
+        if (pid <= 0)
+            continue;
+#ifdef HAVE_WAITID
+        if (waitid_works) {
+            /* The child might have been reaped by the block above in another thread,
+             * so first check if it's ready and, if it is, lock it */
+            if (!isChildReady(pid, &info) ||
+                    !ffd_atomic_compare_exchange(&children.entries[i].pid, &pid, -1,
+                                                 FFD_ATOMIC_RELAXED, FFD_ATOMIC_RELAXED))
+                continue;
+        }
 #endif
+        if (tryReaping(pid, &payload)) {
+            /* this is our child, send notification and free up this entry */
+            notifyAndFreeInfo(&children.header, &children.entries[i], &payload);
+        }
+    }
 
-        for (i = 0; i < (int)sizeofarray(children.entries); ++i) {
-            int pid = ffd_atomic_load(&children.entries[i].pid, FFD_ATOMIC_ACQUIRE);
+    /* try the arrays */
+    array = ffd_atomic_load(&children.header.nextArray, FFD_ATOMIC_ACQUIRE);
+    while (array != NULL) {
+        for (i = 0; i < (int)sizeofarray(array->entries); ++i) {
+            int pid = ffd_atomic_load(&array->entries[i].pid, FFD_ATOMIC_ACQUIRE);
             if (pid <= 0)
                 continue;
 #ifdef HAVE_WAITID
@@ -384,42 +413,18 @@ search_arrays:
                 /* The child might have been reaped by the block above in another thread,
                  * so first check if it's ready and, if it is, lock it */
                 if (!isChildReady(pid, &info) ||
-                        !ffd_atomic_compare_exchange(&children.entries[i].pid, &pid, -1,
+                        !ffd_atomic_compare_exchange(&array->entries[i].pid, &pid, -1,
                                                      FFD_ATOMIC_RELAXED, FFD_ATOMIC_RELAXED))
                     continue;
             }
 #endif
             if (tryReaping(pid, &payload)) {
                 /* this is our child, send notification and free up this entry */
-                notifyAndFreeInfo(&children.header, &children.entries[i], &payload);
+                notifyAndFreeInfo(&array->header, &array->entries[i], &payload);
             }
         }
 
-        /* try the arrays */
-        array = ffd_atomic_load(&children.header.nextArray, FFD_ATOMIC_ACQUIRE);
-        while (array != NULL) {
-            for (i = 0; i < (int)sizeofarray(array->entries); ++i) {
-                int pid = ffd_atomic_load(&array->entries[i].pid, FFD_ATOMIC_ACQUIRE);
-                if (pid <= 0)
-                    continue;
-#ifdef HAVE_WAITID
-                if (waitid_works) {
-                    /* The child might have been reaped by the block above in another thread,
-                     * so first check if it's ready and, if it is, lock it */
-                    if (!isChildReady(pid, &info) ||
-                            !ffd_atomic_compare_exchange(&array->entries[i].pid, &pid, -1,
-                                                         FFD_ATOMIC_RELAXED, FFD_ATOMIC_RELAXED))
-                        continue;
-                }
-#endif
-                if (tryReaping(pid, &payload)) {
-                    /* this is our child, send notification and free up this entry */
-                    notifyAndFreeInfo(&array->header, &array->entries[i], &payload);
-                }
-            }
-
-            array = ffd_atomic_load(&array->header.nextArray, FFD_ATOMIC_ACQUIRE);
-        }
+        array = ffd_atomic_load(&array->header.nextArray, FFD_ATOMIC_ACQUIRE);
     }
 }