GSubprocessLauncher: allow to close passed FDs

By default, when using g_subprocess_launcher_take_fd() to pass an
FD to a child, the GSubprocessLauncher object also takes ownership
of the FD in the parent, and closes it during finalize(). This is
a reasonable assumption in the majority of the cases, but sometimes
it isn't a good idea.

An example is when creating a GSubprocessLauncher in JavaScript:
here, the destruction process is managed by the Garbage Collector,
which means that those sockets will remain opened for some time
after all the references to the object has been droped. This means
that it could be not possible to detect when the child has closed
that same FD, because in order to make that work, both FDs
instances (the one in the parent and the one in the children) must
be closed. This can be a problem in, as an example, a process that
launches a child that communicates with Wayland using an specific
socket (like when using the new API MetaWaylandClient).

Of course, it isn't a valid solution to manually call close() in
the parent process just after the call to spawn(), because the FD
number could be reused in the time between it is manually closed,
and when the object is destroyed and closes again that FD. If that
happens, it will close an incorrect FD.

One solution could be to call run_dispose() from Javascript on the
GSubprocessLauncher object, to force freeing the resources.
Unfortunately, the current code frees them in the finalize()
method, not in dispose() (this is fixed in !1670 (merged) ) but it
isn't a very elegant solution.

This proposal adds a new method, g_subprocess_launcher_close(),
that allows to close the FDs passed to the child. To avoid problems,
after closing an FD with this method, no more spawns are allowed.

Fix: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1677
This commit is contained in:
Sergio Costas 2020-10-04 14:34:41 +02:00
parent 8cefc0ae1e
commit c12762a091
5 changed files with 102 additions and 26 deletions

View File

@ -4617,6 +4617,7 @@ g_subprocess_launcher_take_stdout_fd
g_subprocess_launcher_set_stderr_file_path
g_subprocess_launcher_take_stderr_fd
g_subprocess_launcher_take_fd
g_subprocess_launcher_close
g_subprocess_launcher_set_child_setup
<SUBSECTION Standard>
G_IS_SUBPROCESS_LAUNCHER

View File

@ -44,6 +44,7 @@ struct _GSubprocessLauncher
GArray *basic_fd_assignments;
GArray *needdup_fd_assignments;
gboolean closed_fd;
GSpawnChildSetupFunc child_setup_func;
gpointer child_setup_user_data;

View File

@ -46,6 +46,7 @@
#include "gioenumtypes.h"
#include "gsubprocess.h"
#include "ginitable.h"
#include "gioerror.h"
#ifdef G_OS_UNIX
#include <unistd.h>
@ -136,36 +137,11 @@ g_subprocess_launcher_dispose (GObject *object)
GSubprocessLauncher *self = G_SUBPROCESS_LAUNCHER (object);
#ifdef G_OS_UNIX
guint i;
g_clear_pointer (&self->stdin_path, g_free);
g_clear_pointer (&self->stdout_path, g_free);
g_clear_pointer (&self->stderr_path, g_free);
if (self->stdin_fd != -1)
close (self->stdin_fd);
self->stdin_fd = -1;
if (self->stdout_fd != -1)
close (self->stdout_fd);
self->stdout_fd = -1;
if (self->stderr_fd != -1)
close (self->stderr_fd);
self->stderr_fd = -1;
if (self->basic_fd_assignments)
{
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_subprocess_launcher_close (self);
if (self->child_setup_destroy_notify)
(* self->child_setup_destroy_notify) (self->child_setup_user_data);
@ -652,6 +628,58 @@ g_subprocess_launcher_take_fd (GSubprocessLauncher *self,
}
}
/**
* g_subprocess_launcher_close:
* @self: a #GSubprocessLauncher
*
* Closes all the file descriptors previously passed to the object with
* g_subprocess_launcher_take_fd(), g_subprocess_launcher_take_stderr_fd(), etc.
*
* After calling this method, any subsequent calls to g_subprocess_launcher_spawn() or g_subprocess_launcher_spawnv() will
* return %G_IO_ERROR_CLOSED. This method is idempotent if
* called more than once.
*
* This function is called automatically when the #GSubprocessLauncher
* is disposed, but is provided separately so that garbage collected
* language bindings can call it earlier to guarantee when FDs are closed.
*
* Since: 2.68
*/
void
g_subprocess_launcher_close (GSubprocessLauncher *self)
{
guint i;
g_return_if_fail (G_IS_SUBPROCESS_LAUNCHER (self));
if (self->stdin_fd != -1)
close (self->stdin_fd);
self->stdin_fd = -1;
if (self->stdout_fd != -1)
close (self->stdout_fd);
self->stdout_fd = -1;
if (self->stderr_fd != -1)
close (self->stderr_fd);
self->stderr_fd = -1;
if (self->basic_fd_assignments)
{
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);
}
self->closed_fd = TRUE;
}
/**
* g_subprocess_launcher_set_child_setup: (skip)
* @self: a #GSubprocessLauncher
@ -754,6 +782,17 @@ g_subprocess_launcher_spawnv (GSubprocessLauncher *launcher,
g_return_val_if_fail (argv != NULL && argv[0] != NULL && argv[0][0] != '\0', NULL);
#ifdef G_OS_UNIX
if (launcher->closed_fd)
{
g_set_error (error,
G_IO_ERROR,
G_IO_ERROR_CLOSED,
"Can't spawn a new child because a passed file descriptor has been closed.");
return NULL;
}
#endif
subprocess = g_object_new (G_TYPE_SUBPROCESS,
"argv", argv,
"flags", launcher->flags,

View File

@ -103,6 +103,9 @@ void g_subprocess_launcher_take_fd (GSubpro
gint source_fd,
gint target_fd);
GLIB_AVAILABLE_IN_2_68
void g_subprocess_launcher_close (GSubprocessLauncher *self);
/* Child setup, only available on UNIX */
GLIB_AVAILABLE_IN_2_40
void g_subprocess_launcher_set_child_setup (GSubprocessLauncher *self,

View File

@ -6,6 +6,8 @@
#include <glib-unix.h>
#include <gio/gunixinputstream.h>
#include <gio/gfiledescriptorbased.h>
#include <unistd.h>
#include <fcntl.h>
#endif
/* We write 2^1 + 2^2 ... + 2^10 or 2047 copies of "Hello World!\n"
@ -1483,6 +1485,35 @@ test_cwd (void)
g_object_unref (launcher);
}
#ifdef G_OS_UNIX
static void
test_subprocess_launcher_close (void)
{
GError *local_error = NULL;
GError **error = &local_error;
GSubprocessLauncher *launcher;
GSubprocess *proc;
GPtrArray *args;
int fd;
gboolean is_open;
fd = dup(0);
launcher = g_subprocess_launcher_new (G_SUBPROCESS_FLAGS_NONE);
g_subprocess_launcher_take_fd (launcher, fd, fd);
is_open = fcntl (fd, F_GETFD) != -1;
g_assert_true (is_open);
g_subprocess_launcher_close (launcher);
is_open = fcntl (fd, F_GETFD) != -1;
g_assert_false (is_open);
args = get_test_subprocess_args ("cat", NULL);
proc = g_subprocess_launcher_spawnv (launcher, (const gchar * const *) args->pdata, error);
g_ptr_array_free (args, TRUE);
g_assert_null (proc);
g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_CLOSED);
g_clear_error (error);
g_object_unref (launcher);
}
static void
test_stdout_file (void)
{
@ -1835,6 +1866,7 @@ main (int argc, char **argv)
g_test_add_func ("/gsubprocess/env/inherit", test_env_inherit);
g_test_add_func ("/gsubprocess/cwd", test_cwd);
#ifdef G_OS_UNIX
g_test_add_func ("/gsubprocess/launcher-close", test_subprocess_launcher_close);
g_test_add_func ("/gsubprocess/stdout-file", test_stdout_file);
g_test_add_func ("/gsubprocess/stdout-fd", test_stdout_fd);
g_test_add_func ("/gsubprocess/child-setup", test_child_setup);