mirror of
https://gitlab.gnome.org/GNOME/glib.git
synced 2024-09-20 01:06:15 +02:00
gsubprocess: Use new source/target FD mapping functionality in g_spawn()
This improves performance by eliminating the use of a `GSpawnChildSetupFunc` in the common case (since that setup code has now moved into `g_spawn*()` itself), and enables the use of the fix to avoid the child error reporting FD being overwritten by target FD mappings, introduced via `g_spawn_async_with_pipes_and_fds()`. It reworks how the source/target FD mapping is stored within `GSubprocessLauncher` to match what `g_spawn*()` uses. The two approaches are equivalent. Signed-off-by: Philip Withnall <pwithnall@endlessos.org> Fixes: #2097
This commit is contained in:
Philip Withnall
parent
b31f3f5f80
commit
67a589e505
@ -179,160 +179,6 @@ enum
|
||||
N_PROPS
|
||||
};
|
||||
|
||||
#ifdef G_OS_UNIX
|
||||
typedef struct
|
||||
{
|
||||
gint fds[3];
|
||||
GSpawnChildSetupFunc child_setup_func;
|
||||
gpointer child_setup_data;
|
||||
GArray *basic_fd_assignments;
|
||||
GArray *needdup_fd_assignments;
|
||||
} ChildData;
|
||||
|
||||
static void
|
||||
unset_cloexec (int fd)
|
||||
{
|
||||
int flags;
|
||||
int result;
|
||||
|
||||
flags = fcntl (fd, F_GETFD, 0);
|
||||
|
||||
if (flags != -1)
|
||||
{
|
||||
int errsv;
|
||||
flags &= (~FD_CLOEXEC);
|
||||
do
|
||||
{
|
||||
result = fcntl (fd, F_SETFD, flags);
|
||||
errsv = errno;
|
||||
}
|
||||
while (result == -1 && errsv == EINTR);
|
||||
}
|
||||
}
|
||||
|
||||
static int
|
||||
dupfd_cloexec (int parent_fd)
|
||||
{
|
||||
int fd, errsv;
|
||||
#ifdef F_DUPFD_CLOEXEC
|
||||
do
|
||||
{
|
||||
fd = fcntl (parent_fd, F_DUPFD_CLOEXEC, 3);
|
||||
errsv = errno;
|
||||
}
|
||||
while (fd == -1 && errsv == EINTR);
|
||||
#else
|
||||
/* OS X Snow Lion and earlier don't have F_DUPFD_CLOEXEC:
|
||||
* https://bugzilla.gnome.org/show_bug.cgi?id=710962
|
||||
*/
|
||||
int result, flags;
|
||||
do
|
||||
{
|
||||
fd = fcntl (parent_fd, F_DUPFD, 3);
|
||||
errsv = errno;
|
||||
}
|
||||
while (fd == -1 && errsv == EINTR);
|
||||
flags = fcntl (fd, F_GETFD, 0);
|
||||
if (flags != -1)
|
||||
{
|
||||
flags |= FD_CLOEXEC;
|
||||
do
|
||||
{
|
||||
result = fcntl (fd, F_SETFD, flags);
|
||||
errsv = errno;
|
||||
}
|
||||
while (result == -1 && errsv == EINTR);
|
||||
}
|
||||
#endif
|
||||
return fd;
|
||||
}
|
||||
|
||||
/*
|
||||
* Based on code derived from
|
||||
* gnome-terminal:src/terminal-screen.c:terminal_screen_child_setup(),
|
||||
* used under the LGPLv2+ with permission from author.
|
||||
*/
|
||||
static void
|
||||
child_setup (gpointer user_data)
|
||||
{
|
||||
ChildData *child_data = user_data;
|
||||
guint i;
|
||||
gint result;
|
||||
int errsv;
|
||||
|
||||
/* We're on the child side now. "Rename" the file descriptors in
|
||||
* child_data.fds[] to stdin/stdout/stderr.
|
||||
*
|
||||
* We don't close the originals. It's possible that the originals
|
||||
* should not be closed and if they should be closed then they should
|
||||
* have been created O_CLOEXEC.
|
||||
*/
|
||||
for (i = 0; i < 3; i++)
|
||||
if (child_data->fds[i] != -1 && child_data->fds[i] != (gint) i)
|
||||
{
|
||||
do
|
||||
{
|
||||
result = dup2 (child_data->fds[i], i);
|
||||
errsv = errno;
|
||||
}
|
||||
while (result == -1 && errsv == EINTR);
|
||||
}
|
||||
|
||||
/* Basic fd assignments we can just unset FD_CLOEXEC */
|
||||
if (child_data->basic_fd_assignments)
|
||||
{
|
||||
for (i = 0; i < child_data->basic_fd_assignments->len; i++)
|
||||
{
|
||||
gint fd = g_array_index (child_data->basic_fd_assignments, int, i);
|
||||
|
||||
unset_cloexec (fd);
|
||||
}
|
||||
}
|
||||
|
||||
/* If we're doing remapping fd assignments, we need to handle
|
||||
* the case where the user has specified e.g.:
|
||||
* 5 -> 4, 4 -> 6
|
||||
*
|
||||
* We do this by duping the source fds temporarily.
|
||||
*/
|
||||
if (child_data->needdup_fd_assignments)
|
||||
{
|
||||
for (i = 0; i < child_data->needdup_fd_assignments->len; i += 2)
|
||||
{
|
||||
gint parent_fd = g_array_index (child_data->needdup_fd_assignments, int, i);
|
||||
gint new_parent_fd;
|
||||
|
||||
new_parent_fd = dupfd_cloexec (parent_fd);
|
||||
|
||||
g_array_index (child_data->needdup_fd_assignments, int, i) = new_parent_fd;
|
||||
}
|
||||
for (i = 0; i < child_data->needdup_fd_assignments->len; i += 2)
|
||||
{
|
||||
gint parent_fd = g_array_index (child_data->needdup_fd_assignments, int, i);
|
||||
gint child_fd = g_array_index (child_data->needdup_fd_assignments, int, i+1);
|
||||
|
||||
if (parent_fd == child_fd)
|
||||
{
|
||||
unset_cloexec (parent_fd);
|
||||
}
|
||||
else
|
||||
{
|
||||
do
|
||||
{
|
||||
result = dup2 (parent_fd, child_fd);
|
||||
errsv = errno;
|
||||
}
|
||||
while (result == -1 && errsv == EINTR);
|
||||
(void) close (parent_fd);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (child_data->child_setup_func)
|
||||
child_data->child_setup_func (child_data->child_setup_data);
|
||||
}
|
||||
#endif
|
||||
|
||||
static GInputStream *
|
||||
platform_input_stream_from_spawn_fd (gint fd)
|
||||
{
|
||||
@ -450,12 +296,12 @@ initable_init (GInitable *initable,
|
||||
GError **error)
|
||||
{
|
||||
GSubprocess *self = G_SUBPROCESS (initable);
|
||||
#ifdef G_OS_UNIX
|
||||
ChildData child_data = { { -1, -1, -1 }, 0, NULL, NULL, NULL };
|
||||
#endif
|
||||
gint *pipe_ptrs[3] = { NULL, NULL, NULL };
|
||||
gint pipe_fds[3] = { -1, -1, -1 };
|
||||
gint close_fds[3] = { -1, -1, -1 };
|
||||
#ifdef G_OS_UNIX
|
||||
gint stdin_fd = -1, stdout_fd = -1, stderr_fd = -1;
|
||||
#endif
|
||||
GSpawnFlags spawn_flags = 0;
|
||||
gboolean success = FALSE;
|
||||
gint i;
|
||||
@ -480,11 +326,11 @@ initable_init (GInitable *initable,
|
||||
else if (self->launcher)
|
||||
{
|
||||
if (self->launcher->stdin_fd != -1)
|
||||
child_data.fds[0] = self->launcher->stdin_fd;
|
||||
stdin_fd = self->launcher->stdin_fd;
|
||||
else if (self->launcher->stdin_path != NULL)
|
||||
{
|
||||
child_data.fds[0] = close_fds[0] = unix_open_file (self->launcher->stdin_path, O_RDONLY, error);
|
||||
if (child_data.fds[0] == -1)
|
||||
stdin_fd = close_fds[0] = unix_open_file (self->launcher->stdin_path, O_RDONLY, error);
|
||||
if (stdin_fd == -1)
|
||||
goto out;
|
||||
}
|
||||
}
|
||||
@ -499,11 +345,11 @@ initable_init (GInitable *initable,
|
||||
else if (self->launcher)
|
||||
{
|
||||
if (self->launcher->stdout_fd != -1)
|
||||
child_data.fds[1] = self->launcher->stdout_fd;
|
||||
stdout_fd = self->launcher->stdout_fd;
|
||||
else if (self->launcher->stdout_path != NULL)
|
||||
{
|
||||
child_data.fds[1] = close_fds[1] = unix_open_file (self->launcher->stdout_path, O_CREAT | O_WRONLY, error);
|
||||
if (child_data.fds[1] == -1)
|
||||
stdout_fd = close_fds[1] = unix_open_file (self->launcher->stdout_path, O_CREAT | O_WRONLY, error);
|
||||
if (stdout_fd == -1)
|
||||
goto out;
|
||||
}
|
||||
}
|
||||
@ -516,29 +362,21 @@ initable_init (GInitable *initable,
|
||||
pipe_ptrs[2] = &pipe_fds[2];
|
||||
#ifdef G_OS_UNIX
|
||||
else if (self->flags & G_SUBPROCESS_FLAGS_STDERR_MERGE)
|
||||
/* This will work because stderr gets setup after stdout. */
|
||||
child_data.fds[2] = 1;
|
||||
/* This will work because stderr gets set up after stdout. */
|
||||
stderr_fd = 1;
|
||||
else if (self->launcher)
|
||||
{
|
||||
if (self->launcher->stderr_fd != -1)
|
||||
child_data.fds[2] = self->launcher->stderr_fd;
|
||||
stderr_fd = self->launcher->stderr_fd;
|
||||
else if (self->launcher->stderr_path != NULL)
|
||||
{
|
||||
child_data.fds[2] = close_fds[2] = unix_open_file (self->launcher->stderr_path, O_CREAT | O_WRONLY, error);
|
||||
if (child_data.fds[2] == -1)
|
||||
stderr_fd = close_fds[2] = unix_open_file (self->launcher->stderr_path, O_CREAT | O_WRONLY, error);
|
||||
if (stderr_fd == -1)
|
||||
goto out;
|
||||
}
|
||||
}
|
||||
#endif
|
||||
|
||||
#ifdef G_OS_UNIX
|
||||
if (self->launcher)
|
||||
{
|
||||
child_data.basic_fd_assignments = self->launcher->basic_fd_assignments;
|
||||
child_data.needdup_fd_assignments = self->launcher->needdup_fd_assignments;
|
||||
}
|
||||
#endif
|
||||
|
||||
/* argv0 has no '/' in it? We better do a PATH lookup. */
|
||||
if (strchr (self->argv[0], G_DIR_SEPARATOR) == NULL)
|
||||
{
|
||||
@ -554,23 +392,25 @@ initable_init (GInitable *initable,
|
||||
spawn_flags |= G_SPAWN_DO_NOT_REAP_CHILD;
|
||||
spawn_flags |= G_SPAWN_CLOEXEC_PIPES;
|
||||
|
||||
success = g_spawn_async_with_pipes_and_fds (self->launcher ? self->launcher->cwd : NULL,
|
||||
(const gchar * const *) self->argv,
|
||||
(const gchar * const *) (self->launcher ? self->launcher->envp : NULL),
|
||||
spawn_flags,
|
||||
#ifdef G_OS_UNIX
|
||||
child_data.child_setup_func = self->launcher ? self->launcher->child_setup_func : NULL;
|
||||
child_data.child_setup_data = self->launcher ? self->launcher->child_setup_user_data : NULL;
|
||||
#endif
|
||||
|
||||
success = g_spawn_async_with_pipes (self->launcher ? self->launcher->cwd : NULL,
|
||||
self->argv,
|
||||
self->launcher ? self->launcher->envp : NULL,
|
||||
spawn_flags,
|
||||
#ifdef G_OS_UNIX
|
||||
child_setup, &child_data,
|
||||
self->launcher ? self->launcher->child_setup_func : NULL,
|
||||
self->launcher ? self->launcher->child_setup_user_data : NULL,
|
||||
stdin_fd, stdout_fd, stderr_fd,
|
||||
self->launcher ? (const gint *) self->launcher->source_fds->data : NULL,
|
||||
self->launcher ? (const gint *) self->launcher->target_fds->data : NULL,
|
||||
self->launcher ? self->launcher->source_fds->len : 0,
|
||||
#else
|
||||
NULL, NULL,
|
||||
NULL, NULL,
|
||||
-1, -1, -1,
|
||||
NULL, NULL, 0,
|
||||
#endif
|
||||
&self->pid,
|
||||
pipe_ptrs[0], pipe_ptrs[1], pipe_ptrs[2],
|
||||
error);
|
||||
&self->pid,
|
||||
pipe_ptrs[0], pipe_ptrs[1], pipe_ptrs[2],
|
||||
error);
|
||||
g_assert (success == (self->pid != 0));
|
||||
|
||||
{
|
||||
|
||||
@ -42,8 +42,8 @@ struct _GSubprocessLauncher
|
||||
gint stderr_fd;
|
||||
gchar *stderr_path;
|
||||
|
||||
GArray *basic_fd_assignments;
|
||||
GArray *needdup_fd_assignments;
|
||||
GArray *source_fds;
|
||||
GArray *target_fds; /* always the same length as source_fds */
|
||||
gboolean closed_fd;
|
||||
|
||||
GSpawnChildSetupFunc child_setup_func;
|
||||
|
||||
@ -164,8 +164,8 @@ g_subprocess_launcher_init (GSubprocessLauncher *self)
|
||||
self->stdin_fd = -1;
|
||||
self->stdout_fd = -1;
|
||||
self->stderr_fd = -1;
|
||||
self->basic_fd_assignments = g_array_new (FALSE, 0, sizeof (int));
|
||||
self->needdup_fd_assignments = g_array_new (FALSE, 0, sizeof (int));
|
||||
self->source_fds = g_array_new (FALSE, 0, sizeof (int));
|
||||
self->target_fds = g_array_new (FALSE, 0, sizeof (int));
|
||||
#endif
|
||||
}
|
||||
|
||||
@ -613,18 +613,10 @@ g_subprocess_launcher_take_fd (GSubprocessLauncher *self,
|
||||
gint source_fd,
|
||||
gint target_fd)
|
||||
{
|
||||
if (source_fd == target_fd)
|
||||
if (self->source_fds != NULL && self->target_fds != NULL)
|
||||
{
|
||||
if (self->basic_fd_assignments)
|
||||
g_array_append_val (self->basic_fd_assignments, source_fd);
|
||||
}
|
||||
else
|
||||
{
|
||||
if (self->needdup_fd_assignments)
|
||||
{
|
||||
g_array_append_val (self->needdup_fd_assignments, source_fd);
|
||||
g_array_append_val (self->needdup_fd_assignments, target_fd);
|
||||
}
|
||||
g_array_append_val (self->source_fds, source_fd);
|
||||
g_array_append_val (self->target_fds, target_fd);
|
||||
}
|
||||
}
|
||||
|
||||
@ -664,17 +656,18 @@ g_subprocess_launcher_close (GSubprocessLauncher *self)
|
||||
close (self->stderr_fd);
|
||||
self->stderr_fd = -1;
|
||||
|
||||
if (self->basic_fd_assignments)
|
||||
if (self->source_fds)
|
||||
{
|
||||
for (i = 0; i < self->basic_fd_assignments->len; i++)
|
||||
(void) close (g_array_index (self->basic_fd_assignments, int, i));
|
||||
g_clear_pointer (&self->basic_fd_assignments, g_array_unref);
|
||||
}
|
||||
if (self->needdup_fd_assignments)
|
||||
{
|
||||
for (i = 0; i < self->needdup_fd_assignments->len; i += 2)
|
||||
(void) close (g_array_index (self->needdup_fd_assignments, int, i));
|
||||
g_clear_pointer (&self->needdup_fd_assignments, g_array_unref);
|
||||
g_assert (self->target_fds != NULL);
|
||||
g_assert (self->source_fds->len == self->target_fds->len);
|
||||
|
||||
for (i = 0; i < self->source_fds->len; i++)
|
||||
{
|
||||
(void) close (g_array_index (self->source_fds, int, i));
|
||||
(void) close (g_array_index (self->target_fds, int, i));
|
||||
}
|
||||
g_clear_pointer (&self->source_fds, g_array_unref);
|
||||
g_clear_pointer (&self->target_fds, g_array_unref);
|
||||
}
|
||||
|
||||
self->closed_fd = TRUE;
|
||||
|
||||
Loading…
Reference in New Issue
Block a user