Merge branch '2097-spawn-fd-rewriting' into 'master'

Resolve "GSubprocessLauncher with FD assignment can clash with g_spawn_async internal pipe"

Closes #2097

See merge request GNOME/glib!1690
This commit is contained in:
Philip Withnall 2021-02-16 14:04:39 +00:00
commit 51e964849b
8 changed files with 850 additions and 667 deletions

View File

@ -1527,6 +1527,7 @@ GSpawnFlags
GSpawnChildSetupFunc GSpawnChildSetupFunc
g_spawn_async_with_fds g_spawn_async_with_fds
g_spawn_async_with_pipes g_spawn_async_with_pipes
g_spawn_async_with_pipes_and_fds
g_spawn_async g_spawn_async
g_spawn_sync g_spawn_sync
G_SPAWN_EXIT_ERROR G_SPAWN_EXIT_ERROR

View File

@ -179,160 +179,6 @@ enum
N_PROPS 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 * static GInputStream *
platform_input_stream_from_spawn_fd (gint fd) platform_input_stream_from_spawn_fd (gint fd)
{ {
@ -450,12 +296,12 @@ initable_init (GInitable *initable,
GError **error) GError **error)
{ {
GSubprocess *self = G_SUBPROCESS (initable); 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_ptrs[3] = { NULL, NULL, NULL };
gint pipe_fds[3] = { -1, -1, -1 }; gint pipe_fds[3] = { -1, -1, -1 };
gint close_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; GSpawnFlags spawn_flags = 0;
gboolean success = FALSE; gboolean success = FALSE;
gint i; gint i;
@ -480,11 +326,11 @@ initable_init (GInitable *initable,
else if (self->launcher) else if (self->launcher)
{ {
if (self->launcher->stdin_fd != -1) 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) else if (self->launcher->stdin_path != NULL)
{ {
child_data.fds[0] = close_fds[0] = unix_open_file (self->launcher->stdin_path, O_RDONLY, error); stdin_fd = close_fds[0] = unix_open_file (self->launcher->stdin_path, O_RDONLY, error);
if (child_data.fds[0] == -1) if (stdin_fd == -1)
goto out; goto out;
} }
} }
@ -499,11 +345,11 @@ initable_init (GInitable *initable,
else if (self->launcher) else if (self->launcher)
{ {
if (self->launcher->stdout_fd != -1) 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) 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); stdout_fd = close_fds[1] = unix_open_file (self->launcher->stdout_path, O_CREAT | O_WRONLY, error);
if (child_data.fds[1] == -1) if (stdout_fd == -1)
goto out; goto out;
} }
} }
@ -516,29 +362,21 @@ initable_init (GInitable *initable,
pipe_ptrs[2] = &pipe_fds[2]; pipe_ptrs[2] = &pipe_fds[2];
#ifdef G_OS_UNIX #ifdef G_OS_UNIX
else if (self->flags & G_SUBPROCESS_FLAGS_STDERR_MERGE) else if (self->flags & G_SUBPROCESS_FLAGS_STDERR_MERGE)
/* This will work because stderr gets setup after stdout. */ /* This will work because stderr gets set up after stdout. */
child_data.fds[2] = 1; stderr_fd = 1;
else if (self->launcher) else if (self->launcher)
{ {
if (self->launcher->stderr_fd != -1) 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) 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); stderr_fd = close_fds[2] = unix_open_file (self->launcher->stderr_path, O_CREAT | O_WRONLY, error);
if (child_data.fds[2] == -1) if (stderr_fd == -1)
goto out; goto out;
} }
} }
#endif #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. */ /* argv0 has no '/' in it? We better do a PATH lookup. */
if (strchr (self->argv[0], G_DIR_SEPARATOR) == NULL) 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_DO_NOT_REAP_CHILD;
spawn_flags |= G_SPAWN_CLOEXEC_PIPES; 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 #ifdef G_OS_UNIX
child_data.child_setup_func = self->launcher ? self->launcher->child_setup_func : NULL; self->launcher ? self->launcher->child_setup_func : NULL,
child_data.child_setup_data = self->launcher ? self->launcher->child_setup_user_data : NULL; self->launcher ? self->launcher->child_setup_user_data : NULL,
#endif stdin_fd, stdout_fd, stderr_fd,
self->launcher ? (const gint *) self->launcher->source_fds->data : NULL,
success = g_spawn_async_with_pipes (self->launcher ? self->launcher->cwd : NULL, self->launcher ? (const gint *) self->launcher->target_fds->data : NULL,
self->argv, self->launcher ? self->launcher->source_fds->len : 0,
self->launcher ? self->launcher->envp : NULL,
spawn_flags,
#ifdef G_OS_UNIX
child_setup, &child_data,
#else #else
NULL, NULL, NULL, NULL,
-1, -1, -1,
NULL, NULL, 0,
#endif #endif
&self->pid, &self->pid,
pipe_ptrs[0], pipe_ptrs[1], pipe_ptrs[2], pipe_ptrs[0], pipe_ptrs[1], pipe_ptrs[2],
error); error);
g_assert (success == (self->pid != 0)); g_assert (success == (self->pid != 0));
{ {

View File

@ -42,8 +42,8 @@ struct _GSubprocessLauncher
gint stderr_fd; gint stderr_fd;
gchar *stderr_path; gchar *stderr_path;
GArray *basic_fd_assignments; GArray *source_fds;
GArray *needdup_fd_assignments; GArray *target_fds; /* always the same length as source_fds */
gboolean closed_fd; gboolean closed_fd;
GSpawnChildSetupFunc child_setup_func; GSpawnChildSetupFunc child_setup_func;

View File

@ -164,8 +164,8 @@ g_subprocess_launcher_init (GSubprocessLauncher *self)
self->stdin_fd = -1; self->stdin_fd = -1;
self->stdout_fd = -1; self->stdout_fd = -1;
self->stderr_fd = -1; self->stderr_fd = -1;
self->basic_fd_assignments = g_array_new (FALSE, 0, sizeof (int)); self->source_fds = g_array_new (FALSE, 0, sizeof (int));
self->needdup_fd_assignments = g_array_new (FALSE, 0, sizeof (int)); self->target_fds = g_array_new (FALSE, 0, sizeof (int));
#endif #endif
} }
@ -613,18 +613,10 @@ g_subprocess_launcher_take_fd (GSubprocessLauncher *self,
gint source_fd, gint source_fd,
gint target_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->source_fds, source_fd);
g_array_append_val (self->basic_fd_assignments, source_fd); g_array_append_val (self->target_fds, target_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);
}
} }
} }
@ -664,17 +656,18 @@ g_subprocess_launcher_close (GSubprocessLauncher *self)
close (self->stderr_fd); close (self->stderr_fd);
self->stderr_fd = -1; self->stderr_fd = -1;
if (self->basic_fd_assignments) if (self->source_fds)
{ {
for (i = 0; i < self->basic_fd_assignments->len; i++) g_assert (self->target_fds != NULL);
(void) close (g_array_index (self->basic_fd_assignments, int, i)); g_assert (self->source_fds->len == self->target_fds->len);
g_clear_pointer (&self->basic_fd_assignments, g_array_unref);
} for (i = 0; i < self->source_fds->len; i++)
if (self->needdup_fd_assignments) {
{ (void) close (g_array_index (self->source_fds, int, i));
for (i = 0; i < self->needdup_fd_assignments->len; i += 2) (void) close (g_array_index (self->target_fds, int, i));
(void) close (g_array_index (self->needdup_fd_assignments, int, i)); }
g_clear_pointer (&self->needdup_fd_assignments, g_array_unref); g_clear_pointer (&self->source_fds, g_array_unref);
g_clear_pointer (&self->target_fds, g_array_unref);
} }
self->closed_fd = TRUE; self->closed_fd = TRUE;

View File

@ -191,8 +191,8 @@ protect_argv_string (const gchar *string)
} }
static gint static gint
protect_argv (gchar **argv, protect_argv (const gchar * const *argv,
gchar ***new_argv) gchar ***new_argv)
{ {
gint i; gint i;
gint argc = 0; gint argc = 0;
@ -230,7 +230,7 @@ g_spawn_async (const gchar *working_directory,
GSpawnFlags flags, GSpawnFlags flags,
GSpawnChildSetupFunc child_setup, GSpawnChildSetupFunc child_setup,
gpointer user_data, gpointer user_data,
GPid *child_handle, GPid *child_pid,
GError **error) GError **error)
{ {
g_return_val_if_fail (argv != NULL, FALSE); g_return_val_if_fail (argv != NULL, FALSE);
@ -240,7 +240,7 @@ g_spawn_async (const gchar *working_directory,
flags, flags,
child_setup, child_setup,
user_data, user_data,
child_handle, child_pid,
NULL, NULL, NULL, NULL, NULL, NULL,
error); error);
} }
@ -398,10 +398,10 @@ set_child_error (gintptr report[2],
} }
static gboolean static gboolean
utf8_charv_to_wcharv (char **utf8_charv, utf8_charv_to_wcharv (const gchar * const *utf8_charv,
wchar_t ***wcharv, wchar_t ***wcharv,
int *error_index, int *error_index,
GError **error) GError **error)
{ {
wchar_t **retval = NULL; wchar_t **retval = NULL;
@ -436,16 +436,16 @@ utf8_charv_to_wcharv (char **utf8_charv,
static gboolean static gboolean
do_spawn_directly (gint *exit_status, do_spawn_directly (gint *exit_status,
gboolean do_return_handle, gboolean do_return_handle,
GSpawnFlags flags, GSpawnFlags flags,
gchar **argv, const gchar * const *argv,
char **envp, const gchar * const *envp,
char **protected_argv, const gchar * const *protected_argv,
GPid *child_handle, GPid *child_pid,
GError **error) GError **error)
{ {
const int mode = (exit_status == NULL) ? P_NOWAIT : P_WAIT; const int mode = (exit_status == NULL) ? P_NOWAIT : P_WAIT;
char **new_argv; const gchar * const *new_argv;
gintptr rc = -1; gintptr rc = -1;
int errsv; int errsv;
GError *conv_error = NULL; GError *conv_error = NULL;
@ -515,13 +515,13 @@ do_spawn_directly (gint *exit_status,
if (exit_status == NULL) if (exit_status == NULL)
{ {
if (child_handle && do_return_handle) if (child_pid && do_return_handle)
*child_handle = (GPid) rc; *child_pid = (GPid) rc;
else else
{ {
CloseHandle ((HANDLE) rc); CloseHandle ((HANDLE) rc);
if (child_handle) if (child_pid)
*child_handle = 0; *child_pid = 0;
} }
} }
else else
@ -531,19 +531,23 @@ do_spawn_directly (gint *exit_status,
} }
static gboolean static gboolean
do_spawn_with_fds (gint *exit_status, fork_exec (gint *exit_status,
gboolean do_return_handle, gboolean do_return_handle,
const gchar *working_directory, const gchar *working_directory,
gchar **argv, const gchar * const *argv,
char **envp, const gchar * const *envp,
GSpawnFlags flags, GSpawnFlags flags,
GSpawnChildSetupFunc child_setup, GSpawnChildSetupFunc child_setup,
GPid *child_handle, gpointer user_data,
gint stdin_fd, GPid *child_pid,
gint stdout_fd, gint *stdin_pipe_out,
gint stderr_fd, gint *stdout_pipe_out,
gint *err_report, gint *stderr_pipe_out,
GError **error) gint stdin_fd,
gint stdout_fd,
gint stderr_fd,
gint *err_report,
GError **error)
{ {
char **protected_argv; char **protected_argv;
char args[ARG_COUNT][10]; char args[ARG_COUNT][10];
@ -561,6 +565,13 @@ do_spawn_with_fds (gint *exit_status,
gchar *helper_process; gchar *helper_process;
wchar_t *whelper, **wargv, **wenvp; wchar_t *whelper, **wargv, **wenvp;
gchar *glib_dll_directory; gchar *glib_dll_directory;
int stdin_pipe[2] = { -1, -1 };
int stdout_pipe[2] = { -1, -1 };
int stderr_pipe[2] = { -1, -1 };
g_assert (stdin_pipe_out == NULL || stdin_fd < 0);
g_assert (stdout_pipe_out == NULL || stdout_fd < 0);
g_assert (stderr_pipe_out == NULL || stderr_fd < 0);
if (child_setup && !warned_about_child_setup) if (child_setup && !warned_about_child_setup)
{ {
@ -568,6 +579,27 @@ do_spawn_with_fds (gint *exit_status,
g_warning ("passing a child setup function to the g_spawn functions is pointless on Windows and it is ignored"); g_warning ("passing a child setup function to the g_spawn functions is pointless on Windows and it is ignored");
} }
if (stdin_pipe_out != NULL)
{
if (!make_pipe (stdin_pipe, error))
goto cleanup_and_fail;
stdin_fd = stdin_pipe[0];
}
if (stdout_pipe_out != NULL)
{
if (!make_pipe (stdout_pipe, error))
goto cleanup_and_fail;
stdout_fd = stdout_pipe[1];
}
if (stderr_pipe_out != NULL)
{
if (!make_pipe (stderr_pipe, error))
goto cleanup_and_fail;
stderr_fd = stderr_pipe[1];
}
argc = protect_argv (argv, &protected_argv); argc = protect_argv (argv, &protected_argv);
if (stdin_fd == -1 && stdout_fd == -1 && stderr_fd == -1 && if (stdin_fd == -1 && stdout_fd == -1 && stderr_fd == -1 &&
@ -580,8 +612,8 @@ do_spawn_with_fds (gint *exit_status,
/* We can do without the helper process */ /* We can do without the helper process */
gboolean retval = gboolean retval =
do_spawn_directly (exit_status, do_return_handle, flags, do_spawn_directly (exit_status, do_return_handle, flags,
argv, envp, protected_argv, argv, envp, (const gchar * const *) protected_argv,
child_handle, error); child_pid, error);
g_strfreev (protected_argv); g_strfreev (protected_argv);
return retval; return retval;
} }
@ -714,7 +746,7 @@ do_spawn_with_fds (gint *exit_status,
g_print ("argv[%d]: %s\n", i, (new_argv[i] ? new_argv[i] : "NULL")); g_print ("argv[%d]: %s\n", i, (new_argv[i] ? new_argv[i] : "NULL"));
} }
if (!utf8_charv_to_wcharv (new_argv, &wargv, &conv_error_index, &conv_error)) if (!utf8_charv_to_wcharv ((const gchar * const *) new_argv, &wargv, &conv_error_index, &conv_error))
{ {
if (conv_error_index == ARG_WORKING_DIRECTORY) if (conv_error_index == ARG_WORKING_DIRECTORY)
g_set_error (error, G_SPAWN_ERROR, G_SPAWN_ERROR_CHDIR, g_set_error (error, G_SPAWN_ERROR, G_SPAWN_ERROR_CHDIR,
@ -807,23 +839,23 @@ do_spawn_with_fds (gint *exit_status,
switch (helper_report[0]) switch (helper_report[0])
{ {
case CHILD_NO_ERROR: case CHILD_NO_ERROR:
if (child_handle && do_return_handle) if (child_pid && do_return_handle)
{ {
/* rc is our HANDLE for gspawn-win32-helper. It has /* rc is our HANDLE for gspawn-win32-helper. It has
* told us the HANDLE of its child. Duplicate that into * told us the HANDLE of its child. Duplicate that into
* a HANDLE valid in this process. * a HANDLE valid in this process.
*/ */
if (!DuplicateHandle ((HANDLE) rc, (HANDLE) helper_report[1], if (!DuplicateHandle ((HANDLE) rc, (HANDLE) helper_report[1],
GetCurrentProcess (), (LPHANDLE) child_handle, GetCurrentProcess (), (LPHANDLE) child_pid,
0, TRUE, DUPLICATE_SAME_ACCESS)) 0, TRUE, DUPLICATE_SAME_ACCESS))
{ {
char *emsg = g_win32_error_message (GetLastError ()); char *emsg = g_win32_error_message (GetLastError ());
g_print("%s\n", emsg); g_print("%s\n", emsg);
*child_handle = 0; *child_pid = 0;
} }
} }
else if (child_handle) else if (child_pid)
*child_handle = 0; *child_pid = 0;
write (helper_sync_pipe[1], " ", 1); write (helper_sync_pipe[1], " ", 1);
close_and_invalidate (&helper_sync_pipe[1]); close_and_invalidate (&helper_sync_pipe[1]);
break; break;
@ -840,7 +872,21 @@ do_spawn_with_fds (gint *exit_status,
if (rc != -1) if (rc != -1)
CloseHandle ((HANDLE) rc); CloseHandle ((HANDLE) rc);
/* Close the other process's ends of the pipes in this process,
* otherwise the reader will never get EOF.
*/
close_and_invalidate (&stdin_pipe[0]);
close_and_invalidate (&stdout_pipe[1]);
close_and_invalidate (&stderr_pipe[1]);
if (stdin_pipe_out != NULL)
*stdin_pipe_out = stdin_pipe[1];
if (stdout_pipe_out != NULL)
*stdout_pipe_out = stdout_pipe[0];
if (stderr_pipe_out != NULL)
*stderr_pipe_out = stderr_pipe[0];
return TRUE; return TRUE;
cleanup_and_fail: cleanup_and_fail:
@ -856,70 +902,6 @@ do_spawn_with_fds (gint *exit_status,
if (helper_sync_pipe[1] != -1) if (helper_sync_pipe[1] != -1)
close (helper_sync_pipe[1]); close (helper_sync_pipe[1]);
return FALSE;
}
static gboolean
do_spawn_with_pipes (gint *exit_status,
gboolean do_return_handle,
const gchar *working_directory,
gchar **argv,
char **envp,
GSpawnFlags flags,
GSpawnChildSetupFunc child_setup,
GPid *child_handle,
gint *standard_input,
gint *standard_output,
gint *standard_error,
gint *err_report,
GError **error)
{
int stdin_pipe[2] = { -1, -1 };
int stdout_pipe[2] = { -1, -1 };
int stderr_pipe[2] = { -1, -1 };
if (standard_input && !make_pipe (stdin_pipe, error))
goto cleanup_and_fail;
if (standard_output && !make_pipe (stdout_pipe, error))
goto cleanup_and_fail;
if (standard_error && !make_pipe (stderr_pipe, error))
goto cleanup_and_fail;
if (!do_spawn_with_fds (exit_status,
do_return_handle,
working_directory,
argv,
envp,
flags,
child_setup,
child_handle,
stdin_pipe[0],
stdout_pipe[1],
stderr_pipe[1],
err_report,
error))
goto cleanup_and_fail;
/* Close the other process's ends of the pipes in this process,
* otherwise the reader will never get EOF.
*/
close_and_invalidate (&stdin_pipe[0]);
close_and_invalidate (&stdout_pipe[1]);
close_and_invalidate (&stderr_pipe[1]);
if (standard_input)
*standard_input = stdin_pipe[1];
if (standard_output)
*standard_output = stdout_pipe[0];
if (standard_error)
*standard_error = stderr_pipe[0];
return TRUE;
cleanup_and_fail:
if (stdin_pipe[0] != -1) if (stdin_pipe[0] != -1)
close (stdin_pipe[0]); close (stdin_pipe[0]);
if (stdin_pipe[1] != -1) if (stdin_pipe[1] != -1)
@ -979,20 +961,24 @@ g_spawn_sync (const gchar *working_directory,
if (standard_error) if (standard_error)
*standard_error = NULL; *standard_error = NULL;
if (!do_spawn_with_pipes (&status, if (!fork_exec (&status,
FALSE, FALSE,
working_directory, working_directory,
argv, (const gchar * const *) argv,
envp, (const gchar * const *) envp,
flags, flags,
child_setup, child_setup,
NULL, user_data,
NULL, NULL,
standard_output ? &outpipe : NULL, NULL,
standard_error ? &errpipe : NULL, standard_output ? &outpipe : NULL,
&reportpipe, standard_error ? &errpipe : NULL,
error)) -1,
-1,
-1,
&reportpipe,
error))
return FALSE; return FALSE;
/* Read data from child. */ /* Read data from child. */
@ -1185,7 +1171,7 @@ g_spawn_async_with_pipes (const gchar *working_directory,
GSpawnFlags flags, GSpawnFlags flags,
GSpawnChildSetupFunc child_setup, GSpawnChildSetupFunc child_setup,
gpointer user_data, gpointer user_data,
GPid *child_handle, GPid *child_pid,
gint *standard_input, gint *standard_input,
gint *standard_output, gint *standard_output,
gint *standard_error, gint *standard_error,
@ -1199,20 +1185,24 @@ g_spawn_async_with_pipes (const gchar *working_directory,
/* can't inherit stdin if we have an input pipe. */ /* can't inherit stdin if we have an input pipe. */
g_return_val_if_fail (standard_input == NULL || g_return_val_if_fail (standard_input == NULL ||
!(flags & G_SPAWN_CHILD_INHERITS_STDIN), FALSE); !(flags & G_SPAWN_CHILD_INHERITS_STDIN), FALSE);
return do_spawn_with_pipes (NULL, return fork_exec (NULL,
(flags & G_SPAWN_DO_NOT_REAP_CHILD), (flags & G_SPAWN_DO_NOT_REAP_CHILD),
working_directory, working_directory,
argv, (const gchar * const *) argv,
envp, (const gchar * const *) envp,
flags, flags,
child_setup, child_setup,
child_handle, user_data,
standard_input, child_pid,
standard_output, standard_input,
standard_error, standard_output,
NULL, standard_error,
error); -1,
-1,
-1,
NULL,
error);
} }
gboolean gboolean
@ -1222,7 +1212,7 @@ g_spawn_async_with_fds (const gchar *working_directory,
GSpawnFlags flags, GSpawnFlags flags,
GSpawnChildSetupFunc child_setup, GSpawnChildSetupFunc child_setup,
gpointer user_data, gpointer user_data,
GPid *child_handle, GPid *child_pid,
gint stdin_fd, gint stdin_fd,
gint stdout_fd, gint stdout_fd,
gint stderr_fd, gint stderr_fd,
@ -1237,19 +1227,83 @@ g_spawn_async_with_fds (const gchar *working_directory,
g_return_val_if_fail (stdin_fd == -1 || g_return_val_if_fail (stdin_fd == -1 ||
!(flags & G_SPAWN_CHILD_INHERITS_STDIN), FALSE); !(flags & G_SPAWN_CHILD_INHERITS_STDIN), FALSE);
return do_spawn_with_fds (NULL, return fork_exec (NULL,
(flags & G_SPAWN_DO_NOT_REAP_CHILD), (flags & G_SPAWN_DO_NOT_REAP_CHILD),
working_directory, working_directory,
argv, (const gchar * const *) argv,
envp, (const gchar * const *) envp,
flags, flags,
child_setup, child_setup,
child_handle, user_data,
stdin_fd, child_pid,
stdout_fd, NULL,
stderr_fd, NULL,
NULL, NULL,
error); stdin_fd,
stdout_fd,
stderr_fd,
NULL,
error);
}
gboolean
g_spawn_async_with_pipes_and_fds (const gchar *working_directory,
const gchar * const *argv,
const gchar * const *envp,
GSpawnFlags flags,
GSpawnChildSetupFunc child_setup,
gpointer user_data,
gint stdin_fd,
gint stdout_fd,
gint stderr_fd,
const gint *source_fds,
const gint *target_fds,
gsize n_fds,
GPid *child_pid_out,
gint *stdin_pipe_out,
gint *stdout_pipe_out,
gint *stderr_pipe_out,
GError **error)
{
g_return_val_if_fail (argv != NULL, FALSE);
g_return_val_if_fail (stdout_pipe_out == NULL ||
!(flags & G_SPAWN_STDOUT_TO_DEV_NULL), FALSE);
g_return_val_if_fail (stderr_pipe_out == NULL ||
!(flags & G_SPAWN_STDERR_TO_DEV_NULL), FALSE);
/* can't inherit stdin if we have an input pipe. */
g_return_val_if_fail (stdin_pipe_out == NULL ||
!(flags & G_SPAWN_CHILD_INHERITS_STDIN), FALSE);
/* cant use pipes and stdin/stdout/stderr FDs */
g_return_val_if_fail (stdin_pipe_out == NULL || stdin_fd < 0, FALSE);
g_return_val_if_fail (stdout_pipe_out == NULL || stdout_fd < 0, FALSE);
g_return_val_if_fail (stderr_pipe_out == NULL || stderr_fd < 0, FALSE);
/* source_fds/target_fds isnt supported on Windows at the moment. */
if (n_fds != 0)
{
g_set_error_literal (error, G_SPAWN_ERROR, G_SPAWN_ERROR_INVAL,
"FD redirection is not supported on Windows at the moment");
return FALSE;
}
return fork_exec (NULL,
(flags & G_SPAWN_DO_NOT_REAP_CHILD),
working_directory,
argv,
envp,
flags,
child_setup,
user_data,
child_pid_out,
stdin_pipe_out,
stdout_pipe_out,
stderr_pipe_out,
stdin_fd,
stdout_fd,
stderr_fd,
NULL,
error);
} }
gboolean gboolean
@ -1384,7 +1438,7 @@ g_spawn_async_utf8 (const gchar *working_directory,
GSpawnFlags flags, GSpawnFlags flags,
GSpawnChildSetupFunc child_setup, GSpawnChildSetupFunc child_setup,
gpointer user_data, gpointer user_data,
GPid *child_handle, GPid *child_pid,
GError **error) GError **error)
{ {
return g_spawn_async (working_directory, return g_spawn_async (working_directory,
@ -1393,7 +1447,7 @@ g_spawn_async_utf8 (const gchar *working_directory,
flags, flags,
child_setup, child_setup,
user_data, user_data,
child_handle, child_pid,
error); error);
} }
@ -1404,7 +1458,7 @@ g_spawn_async_with_pipes_utf8 (const gchar *working_directory,
GSpawnFlags flags, GSpawnFlags flags,
GSpawnChildSetupFunc child_setup, GSpawnChildSetupFunc child_setup,
gpointer user_data, gpointer user_data,
GPid *child_handle, GPid *child_pid,
gint *standard_input, gint *standard_input,
gint *standard_output, gint *standard_output,
gint *standard_error, gint *standard_error,
@ -1416,7 +1470,7 @@ g_spawn_async_with_pipes_utf8 (const gchar *working_directory,
flags, flags,
child_setup, child_setup,
user_data, user_data,
child_handle, child_pid,
standard_input, standard_input,
standard_output, standard_output,
standard_error, standard_error,

File diff suppressed because it is too large Load Diff

View File

@ -213,6 +213,25 @@ gboolean g_spawn_async_with_pipes (const gchar *working_directory,
gint *standard_error, gint *standard_error,
GError **error); GError **error);
GLIB_AVAILABLE_IN_2_68
gboolean g_spawn_async_with_pipes_and_fds (const gchar *working_directory,
const gchar * const *argv,
const gchar * const *envp,
GSpawnFlags flags,
GSpawnChildSetupFunc child_setup,
gpointer user_data,
gint stdin_fd,
gint stdout_fd,
gint stderr_fd,
const gint *source_fds,
const gint *target_fds,
gsize n_fds,
GPid *child_pid_out,
gint *stdin_pipe_out,
gint *stdout_pipe_out,
gint *stderr_pipe_out,
GError **error);
/* Lets you provide fds for stdin/stdout/stderr */ /* Lets you provide fds for stdin/stdout/stderr */
GLIB_AVAILABLE_IN_2_58 GLIB_AVAILABLE_IN_2_58
gboolean g_spawn_async_with_fds (const gchar *working_directory, gboolean g_spawn_async_with_fds (const gchar *working_directory,

View File

@ -24,11 +24,16 @@
#include "config.h" #include "config.h"
#include <glib.h> #include <glib.h>
#include <locale.h>
#include <string.h> #include <string.h>
#include <fcntl.h> #include <fcntl.h>
#ifdef G_OS_UNIX #ifdef G_OS_UNIX
#include <glib-unix.h> #include <glib-unix.h>
#include <glib/gstdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#endif #endif
#ifdef G_OS_WIN32 #ifdef G_OS_WIN32
@ -420,6 +425,72 @@ test_spawn_nonexistent (void)
g_clear_error (&error); g_clear_error (&error);
} }
/* Test that FD assignments in a spawned process dont overwrite and break the
* child_err_report_fd which is used to report error information back from the
* intermediate child process to the parent.
*
* https://gitlab.gnome.org/GNOME/glib/-/issues/2097 */
static void
test_spawn_fd_assignment_clash (void)
{
#ifdef G_OS_UNIX
int tmp_fd;
guint i;
const guint n_fds = 10;
gint source_fds[n_fds];
gint target_fds[n_fds];
const gchar *argv[] = { "/nonexistent" };
gboolean retval;
GError *local_error = NULL;
struct stat statbuf;
/* Open a temporary file and duplicate its FD several times so we have several
* FDs to remap in the child process. */
tmp_fd = g_file_open_tmp ("glib-spawn-test-XXXXXX", NULL, NULL);
g_assert_cmpint (tmp_fd, >=, 0);
for (i = 0; i < (n_fds - 1); ++i)
{
int source = fcntl (tmp_fd, F_DUPFD_CLOEXEC, 3);
g_assert_cmpint (source, >=, 0);
source_fds[i] = source;
target_fds[i] = source + n_fds;
}
source_fds[i] = tmp_fd;
target_fds[i] = tmp_fd + n_fds;
/* Print out the FD map. */
g_test_message ("FD map:");
for (i = 0; i < n_fds; i++)
g_test_message (" • %d → %d", source_fds[i], target_fds[i]);
/* Spawn the subprocess. This should fail because the executable doesnt
* exist. */
retval = g_spawn_async_with_pipes_and_fds (NULL, argv, NULL, G_SPAWN_DEFAULT,
NULL, NULL, -1, -1, -1,
source_fds, target_fds, n_fds,
NULL, NULL, NULL, NULL,
&local_error);
g_assert_error (local_error, G_SPAWN_ERROR, G_SPAWN_ERROR_NOENT);
g_assert_false (retval);
g_clear_error (&local_error);
/* Check nothing was written to the temporary file, as would happen if the FD
* mapping was messed up to conflict with the child process error reporting FD.
* See https://gitlab.gnome.org/GNOME/glib/-/issues/2097 */
g_assert_no_errno (fstat (tmp_fd, &statbuf));
g_assert_cmpuint (statbuf.st_size, ==, 0);
/* Clean up. */
for (i = 0; i < n_fds; i++)
g_close (source_fds[i], NULL);
#else /* !G_OS_UNIX */
g_test_skip ("FD redirection only supported on Unix");
#endif /* !G_OS_UNIX */
}
int int
main (int argc, main (int argc,
char *argv[]) char *argv[])
@ -427,6 +498,8 @@ main (int argc,
char *dirname; char *dirname;
int ret; int ret;
setlocale (LC_ALL, "");
g_test_init (&argc, &argv, NULL); g_test_init (&argc, &argv, NULL);
dirname = g_path_get_dirname (argv[0]); dirname = g_path_get_dirname (argv[0]);
@ -453,6 +526,7 @@ main (int argc,
g_test_add_func ("/gthread/spawn-script", test_spawn_script); g_test_add_func ("/gthread/spawn-script", test_spawn_script);
g_test_add_func ("/gthread/spawn/nonexistent", test_spawn_nonexistent); g_test_add_func ("/gthread/spawn/nonexistent", test_spawn_nonexistent);
g_test_add_func ("/gthread/spawn-posix-spawn", test_posix_spawn); g_test_add_func ("/gthread/spawn-posix-spawn", test_posix_spawn);
g_test_add_func ("/gthread/spawn/fd-assignment-clash", test_spawn_fd_assignment_clash);
ret = g_test_run(); ret = g_test_run();