[WPE][GTK] Avoid another child setup function in process launcher code

https://bugs.webkit.org/show_bug.cgi?id=222049

Patch by Michael Catanzaro <mcatanzaro@gnome.org> on 2021-02-18
Reviewed by Carlos Garcia Campos.

Avoiding child setup functions is desirable because it could allow GSubprocess to use
posix_spawn() instead of fork() in the future. That's not possible to do if we have code
that needs to run between fork() and exec().

In this case, the child setup is used only to unset CLOEXEC. We could simply not set it in
the first place. This only fails if a secondary thread decides to launch a subprocess before
XDGDBusProxyLauncher::launch returns. That window already exists in many other places (e.g.
anywhere else setCloseOnExec is called, such as for IPC::Connection objects). Threads should
not do that.

This also fixes a bug where unsetting CLOEXEC would fail if we get unlucky and receive
EINTR. A loop is required here. WTF::setCloseOnExec handles that for us.

* UIProcess/Launcher/glib/BubblewrapLauncher.cpp:
(WebKit::XDGDBusProxyLauncher::launch):
(WebKit::XDGDBusProxyLauncher::childSetupFunc): Deleted.

Canonical link: https://commits.webkit.org/234285@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@273087 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog

@@ -1,3 +1,27 @@
+2021-02-18  Michael Catanzaro  <mcatanzaro@gnome.org>
+
+        [WPE][GTK] Avoid another child setup function in process launcher code
+        https://bugs.webkit.org/show_bug.cgi?id=222049
+
+        Reviewed by Carlos Garcia Campos.
+
+        Avoiding child setup functions is desirable because it could allow GSubprocess to use
+        posix_spawn() instead of fork() in the future. That's not possible to do if we have code
+        that needs to run between fork() and exec().
+
+        In this case, the child setup is used only to unset CLOEXEC. We could simply not set it in
+        the first place. This only fails if a secondary thread decides to launch a subprocess before
+        XDGDBusProxyLauncher::launch returns. That window already exists in many other places (e.g.
+        anywhere else setCloseOnExec is called, such as for IPC::Connection objects). Threads should
+        not do that.
+
+        This also fixes a bug where unsetting CLOEXEC would fail if we get unlucky and receive
+        EINTR. A loop is required here. WTF::setCloseOnExec handles that for us.
+
+        * UIProcess/Launcher/glib/BubblewrapLauncher.cpp:
+        (WebKit::XDGDBusProxyLauncher::launch):
+        (WebKit::XDGDBusProxyLauncher::childSetupFunc): Deleted.
+
 2021-02-18  Youenn Fablet  <youenn@apple.com>
 
         Set ENABLE_VP9 to 1 on IOS

Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp

@@ -27,6 +27,7 @@
 #include <sys/ioctl.h>
 #include <unistd.h>
 #include <wtf/FileSystem.h>
+#include <wtf/UniStdExtras.h>
 #include <wtf/glib/GLibUtilities.h>
 #include <wtf/glib/GRefPtr.h>
 #include <wtf/glib/GUniquePtr.h>
@@ -189,8 +190,9 @@ class XDGDBusProxyLauncher {
             return;
 
         int syncFds[2];
-        if (pipe2(syncFds, O_CLOEXEC) == -1)
+        if (pipe(syncFds) == -1)
             g_error("Failed to make syncfds for dbus-proxy: %s", g_strerror(errno));
+        setCloseOnExec(syncFds[0]);
 
         GUniquePtr<char> syncFdStr(g_strdup_printf("--fd=%d", syncFds[1]));
 
@@ -224,7 +226,6 @@ class XDGDBusProxyLauncher {
         argv[i] = nullptr;
 
         GRefPtr<GSubprocessLauncher> launcher = adoptGRef(g_subprocess_launcher_new(G_SUBPROCESS_FLAGS_INHERIT_FDS));
-        g_subprocess_launcher_set_child_setup(launcher.get(), childSetupFunc, GINT_TO_POINTER(syncFds[1]), nullptr);
         g_subprocess_launcher_take_fd(launcher.get(), proxyFd, proxyFd);
         g_subprocess_launcher_take_fd(launcher.get(), syncFds[1], syncFds[1]);
 
@@ -248,12 +249,6 @@ class XDGDBusProxyLauncher {
     };
 
 private:
-    static void childSetupFunc(gpointer userdata)
-    {
-        int fd = GPOINTER_TO_INT(userdata);
-        fcntl(fd, F_SETFD, 0); // Unset CLOEXEC
-    }
-
     static CString makeProxyPath(const char* appRunDir)
     {
         if (g_mkdir_with_parents(appRunDir, 0700) == -1) {