Commit 0e1a3ee3 authored by Colin Walters's avatar Colin Walters Committed by Allison Karlitskaya

gsubprocess: Fix up communicate

We weren't closing the streams after we were done reading or writing,
which is kind of essential.  The easy way to fix this is to just use
g_output_stream_splice() to a GMemoryOutputStream rather than
hand-rolling it.  This results in a substantial reduction of code
complexity.

A second serious issue is that we were marking the task as complete when
the process exits, but that's racy - there could still be data to read
from stdout.  Fix this by just refcounting outstanding operations.

This code, not surprisingly, looks a lot like the "multi" test.

Next, because processes output binary data, I'd be forced to annotate
the char*/length pairs as (array) (element-type uint8).  But rather than
doing that, it's *far* simpler to just use GBytes.

We need a version of this that actually validates as UTF-8, that will be
in the next patch.
parent 5b48dc40
This diff is collapsed.
......@@ -124,18 +124,14 @@ gint g_subprocess_get_term_sig (GSubprocess *s
GLIB_AVAILABLE_IN_2_40
gboolean g_subprocess_communicate (GSubprocess *subprocess,
const gchar *stdin_data,
gssize stdin_length,
GBytes *stdin_buf,
GCancellable *cancellable,
gchar **stdout_data,
gsize *stdout_length,
gchar **stderr_data,
gsize *stderr_length,
GBytes **stdout_buf,
GBytes **stderr_buf,
GError **error);
GLIB_AVAILABLE_IN_2_40
void g_subprocess_communicate_async (GSubprocess *subprocess,
const gchar *stdin_data,
gssize stdin_length,
GBytes *stdin_buf,
GCancellable *cancellable,
GAsyncReadyCallback callback,
gpointer user_data);
......@@ -143,30 +139,8 @@ void g_subprocess_communicate_async (GSubprocess *s
GLIB_AVAILABLE_IN_2_40
gboolean g_subprocess_communicate_finish (GSubprocess *subprocess,
GAsyncResult *result,
gchar **stdout_data,
gsize *stdout_length,
gchar **stderr_data,
gsize *stderr_length,
GError **error);
GLIB_AVAILABLE_IN_2_40
gboolean g_subprocess_communicate_bytes (GSubprocess *subprocess,
GBytes *stdin_bytes,
GCancellable *cancellable,
GBytes **stdout_bytes,
GBytes **stderr_bytes,
GError **error);
GLIB_AVAILABLE_IN_2_40
void g_subprocess_communicate_bytes_async (GSubprocess *subprocess,
GBytes *stdin_bytes,
GCancellable *cancellable,
GAsyncReadyCallback callback,
gpointer user_data);
GLIB_AVAILABLE_IN_2_40
gboolean g_subprocess_communicate_bytes_finish (GSubprocess *subprocess,
GAsyncResult *result,
GBytes **stdout_bytes,
GBytes **stderr_bytes,
GBytes **stdout_buf,
GBytes **stderr_buf,
GError **error);
G_END_DECLS
......
......@@ -130,6 +130,7 @@ static void
g_subprocess_launcher_finalize (GObject *object)
{
GSubprocessLauncher *self = G_SUBPROCESS_LAUNCHER (object);
guint i;
g_strfreev (self->envp);
g_free (self->cwd);
......@@ -148,8 +149,18 @@ g_subprocess_launcher_finalize (GObject *object)
if (self->stderr_fd != -1)
close (self->stderr_fd);
g_clear_pointer (&self->basic_fd_assignments, g_array_unref);
g_clear_pointer (&self->needdup_fd_assignments, g_array_unref);
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_array_unref (self->basic_fd_assignments);
}
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_array_unref (self->needdup_fd_assignments);
}
#endif
if (self->child_setup_destroy_notify)
......@@ -570,23 +581,26 @@ g_subprocess_launcher_take_stderr_fd (GSubprocessLauncher *self,
}
/**
* g_subprocess_launcher_pass_fd:
* g_subprocess_launcher_take_fd:
* @self: a #GSubprocessLauncher
* @source_fd: File descriptor in parent process
* @target_fd: Target descriptor for child process
*
* Pass an arbitrary file descriptor from parent process to
* the child. By default, all file descriptors from the parent
* will be closed. This function allows you to create (for example)
* a custom pipe() or socketpair() before launching the process, and
* choose the target descriptor in the child.
* Transfer an arbitrary file descriptor from parent process to the
* child. This function takes "ownership" of the fd; it will be closed
* in the parent when @self is freed.
*
* By default, all file descriptors from the parent will be closed.
* This function allows you to create (for example) a custom pipe() or
* socketpair() before launching the process, and choose the target
* descriptor in the child.
*
* An example use case is GNUPG, which has a command line argument
* --passphrase-fd providing a file descriptor number where it expects
* the passphrase to be written.
*/
void
g_subprocess_launcher_pass_fd (GSubprocessLauncher *self,
g_subprocess_launcher_take_fd (GSubprocessLauncher *self,
gint source_fd,
gint target_fd)
{
......
......@@ -101,7 +101,7 @@ void g_subprocess_launcher_take_stderr_fd (GSubpro
gint fd);
GLIB_AVAILABLE_IN_2_40
void g_subprocess_launcher_pass_fd (GSubprocessLauncher *self,
void g_subprocess_launcher_take_fd (GSubprocessLauncher *self,
gint source_fd,
gint target_fd);
......
......@@ -545,6 +545,69 @@ test_multi_1 (void)
g_object_unref (third);
}
typedef struct {
gboolean running;
GError *error;
} TestAsyncCommunicateData;
static void
on_communicate_complete (GObject *proc,
GAsyncResult *result,
gpointer user_data)
{
TestAsyncCommunicateData *data = user_data;
GBytes *stdout;
const guint8 *stdout_data;
gsize stdout_len;
data->running = FALSE;
(void) g_subprocess_communicate_finish ((GSubprocess*)proc, result,
&stdout, NULL, &data->error);
g_assert_no_error (data->error);
stdout_data = g_bytes_get_data (stdout, &stdout_len);
g_assert_cmpint (stdout_len, ==, 11);
g_assert (memcmp (stdout_data, "hello world", 11) == 0);
g_bytes_unref (stdout);
}
static void
test_communicate (void)
{
GError *local_error = NULL;
GError **error = &local_error;
GPtrArray *args;
TestAsyncCommunicateData data = { 0, };
GSubprocess *proc;
GCancellable *cancellable = NULL;
GBytes *input;
args = get_test_subprocess_args ("cat", NULL);
proc = g_subprocess_newv ((const gchar* const*)args->pdata,
G_SUBPROCESS_FLAGS_STDIN_PIPE | G_SUBPROCESS_FLAGS_STDOUT_PIPE,
error);
g_assert_no_error (local_error);
g_ptr_array_free (args, TRUE);
input = g_bytes_new_static ("hello world", strlen ("hello world"));
data.error = local_error;
g_subprocess_communicate_async (proc, input,
cancellable,
on_communicate_complete,
&data);
data.running = TRUE;
while (data.running)
g_main_context_iteration (NULL, TRUE);
g_assert_no_error (local_error);
g_object_unref (proc);
}
static gboolean
send_terminate (gpointer user_data)
{
......@@ -788,14 +851,12 @@ test_pass_fd (void)
args = get_test_subprocess_args ("write-to-fds", basic_fd_str, needdup_fd_str, NULL);
launcher = g_subprocess_launcher_new (G_SUBPROCESS_FLAGS_NONE);
g_subprocess_launcher_pass_fd (launcher, basic_pipefds[1], basic_pipefds[1]);
g_subprocess_launcher_pass_fd (launcher, needdup_pipefds[1], needdup_pipefds[1] + 1);
g_subprocess_launcher_take_fd (launcher, basic_pipefds[1], basic_pipefds[1]);
g_subprocess_launcher_take_fd (launcher, needdup_pipefds[1], needdup_pipefds[1] + 1);
proc = g_subprocess_launcher_spawnv (launcher, (const gchar * const *) args->pdata, error);
g_ptr_array_free (args, TRUE);
g_assert_no_error (local_error);
(void) close (basic_pipefds[1]);
(void) close (needdup_pipefds[1]);
g_free (basic_fd_str);
g_free (needdup_fd_str);
......@@ -843,6 +904,7 @@ main (int argc, char **argv)
g_test_add_func ("/gsubprocess/cat-utf8", test_cat_utf8);
g_test_add_func ("/gsubprocess/cat-eof", test_cat_eof);
g_test_add_func ("/gsubprocess/multi1", test_multi_1);
g_test_add_func ("/gsubprocess/communicate", test_communicate);
g_test_add_func ("/gsubprocess/terminate", test_terminate);
#ifdef G_OS_UNIX
g_test_add_func ("/gsubprocess/stdout-file", test_stdout_file);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment