Title: [273087] trunk/Source/WebKit
- Revision
- 273087
- Author
- commit-qu...@webkit.org
- Date
- 2021-02-18 11:40:50 -0800 (Thu, 18 Feb 2021)
Log Message
[WPE][GTK] Avoid another child setup function in process launcher code
https://bugs.webkit.org/show_bug.cgi?id=222049
Patch by Michael Catanzaro <mcatanz...@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.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (273086 => 273087)
--- trunk/Source/WebKit/ChangeLog 2021-02-18 19:14:34 UTC (rev 273086)
+++ trunk/Source/WebKit/ChangeLog 2021-02-18 19:40:50 UTC (rev 273087)
@@ -1,3 +1,27 @@
+2021-02-18 Michael Catanzaro <mcatanz...@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 <you...@apple.com>
Set ENABLE_VP9 to 1 on IOS
Modified: trunk/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp (273086 => 273087)
--- trunk/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp 2021-02-18 19:14:34 UTC (rev 273086)
+++ trunk/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp 2021-02-18 19:40:50 UTC (rev 273087)
@@ -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 @@
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 @@
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 @@
};
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) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
