libsysprof: provide unwind pipe from client

We don't need a socketpair for this. Additionally, things seem to work
better from the service when the client provides the pipe. Otherwise, when
running as a dbus service I often have issues with things getting closed
out from under us.

src/libsysprof/sysprof-user-sampler.c

@@ -20,6 +20,8 @@
 
 #include "config.h"
 
+#include <fcntl.h>
+
 #include <sys/ioctl.h>
 #include <sys/eventfd.h>
 
@@ -62,7 +64,8 @@ struct _SysprofUserSampler
 {
   SysprofInstrument parent_instance;
   GArray *perf_fds;
-  int capture_fd;
+  int capture_fd_read;
+  int capture_fd_write;
   int event_fd;
   guint stack_size;
 };
@@ -289,17 +292,14 @@ call_unwind_cb (GObject      *object,
 {
   g_autoptr(DexPromise) promise = user_data;
   g_autoptr(GUnixFDList) out_fd_list = NULL;
-  g_autoptr(GVariant) out_capture_fd = NULL;
-  g_autofd int capture_fd = -1;
   GError *error = NULL;
 
   g_assert (IPC_IS_UNWINDER (object));
   g_assert (G_IS_ASYNC_RESULT (result));
   g_assert (DEX_IS_PROMISE (promise));
 
-  if (ipc_unwinder_call_unwind_finish (IPC_UNWINDER (object), &out_capture_fd, &out_fd_list, result, &error) &&
+  if (ipc_unwinder_call_unwind_finish (IPC_UNWINDER (object), &out_fd_list, result, &error))
-      -1 != (capture_fd = g_unix_fd_list_get (out_fd_list, g_variant_get_handle (out_capture_fd), &error)))
+    dex_promise_resolve_boolean (promise, TRUE);
-    promise_resolve_fd (promise, g_steal_fd (&capture_fd));
   else
     dex_promise_reject (promise, error);
 }
@@ -420,31 +420,26 @@ sysprof_user_sampler_prepare_fiber (gpointer user_data)
         {
           g_autoptr(DexPromise) promise = dex_promise_new ();
           int event_fd_handle = g_unix_fd_list_append (fd_list, prepare->sampler->event_fd, NULL);
-          g_autofd int fd = -1;
+          int capture_fd_handle = g_unix_fd_list_append (fd_list, prepare->sampler->capture_fd_write, NULL);
 
           ipc_unwinder_call_unwind (unwinder,
                                     prepare->stack_size,
                                     g_variant_builder_end (&builder),
                                     g_variant_new_handle (event_fd_handle),
+                                    g_variant_new_handle (capture_fd_handle),
                                     fd_list,
                                     NULL,
                                     call_unwind_cb,
                                     dex_ref (promise));
 
-          fd = await_fd (dex_ref (promise), &error);
+          if (!dex_await (dex_ref (promise), &error))
-
-          if (fd == -1)
             {
               _sysprof_recording_diagnostic (prepare->recording,
                                              "Sampler",
-                                             "Failed to setup user-space unwinder: %s",
+                                             "Failed to setup thread unwinder: %s",
                                              error->message);
               g_clear_error (&error);
             }
-          else
-            {
-              prepare->sampler->capture_fd = g_steal_fd (&fd);
-            }
         }
     }
 
@@ -506,21 +501,26 @@ sysprof_user_sampler_record_fiber (gpointer user_data)
 
   writer = _sysprof_recording_writer (record->recording);
 
-  sysprof_user_sampler_ioctl (record->sampler, TRUE);
+  if (record->sampler->capture_fd_read != -1)
+    {
+      sysprof_user_sampler_ioctl (record->sampler, TRUE);
 
-  g_debug ("Staring muxer for capture_fd");
+      g_debug ("Staring muxer for capture_fd %d", record->sampler->capture_fd_read);
-  muxer_source = sysprof_muxer_source_new (g_steal_fd (&record->sampler->capture_fd), writer);
+      muxer_source = sysprof_muxer_source_new (g_steal_fd (&record->sampler->capture_fd_read), writer);
-  g_source_set_static_name (muxer_source, "[stack-muxer]");
+      g_source_set_static_name (muxer_source, "[stack-muxer]");
-  g_source_attach (muxer_source, NULL);
+      g_source_attach (muxer_source, NULL);
 
-  if (!dex_await (dex_ref (record->cancellable), &error))
+      if (!dex_await (dex_ref (record->cancellable), &error))
-    g_debug ("UserSampler shutting down for reason: %s", error->message);
+        g_debug ("UserSampler shutting down for reason: %s", error->message);
 
-  write (record->sampler->event_fd, &exiting, sizeof exiting);
+      write (record->sampler->event_fd, &exiting, sizeof exiting);
 
-  g_source_destroy (muxer_source);
+      g_source_destroy (muxer_source);
 
-  sysprof_user_sampler_ioctl (record->sampler, FALSE);
+      sysprof_user_sampler_ioctl (record->sampler, FALSE);
+    }
+  else
+    g_warning ("No capture FD available for muxing");
 
   return dex_future_new_for_boolean (TRUE);
 }
@@ -555,7 +555,8 @@ sysprof_user_sampler_finalize (GObject *object)
 
   g_clear_pointer (&self->perf_fds, g_array_unref);
 
-  g_clear_fd (&self->capture_fd, NULL);
+  g_clear_fd (&self->capture_fd_read, NULL);
+  g_clear_fd (&self->capture_fd_write, NULL);
   g_clear_fd (&self->event_fd, NULL);
 
   G_OBJECT_CLASS (sysprof_user_sampler_parent_class)->finalize (object);
@@ -577,9 +578,19 @@ sysprof_user_sampler_class_init (SysprofUserSamplerClass *klass)
 static void
 sysprof_user_sampler_init (SysprofUserSampler *self)
 {
-  self->capture_fd = -1;
+  int fds[2];
+
   self->event_fd = eventfd (0, EFD_CLOEXEC);
 
+  self->capture_fd_read = -1;
+  self->capture_fd_write = -1;
+
+  if (pipe2 (fds, O_CLOEXEC) == 0)
+    {
+      self->capture_fd_read = fds[0];
+      self->capture_fd_write = fds[1];
+    }
+
   self->perf_fds = g_array_new (FALSE, FALSE, sizeof (int));
   g_array_set_clear_func (self->perf_fds, close_fd);
 }

src/sysprofd/ipc-unwinder-impl.c

@@ -24,6 +24,7 @@
 #include "config.h"
 
 #include <errno.h>
+#include <fcntl.h>
 
 #include <signal.h>
 #include <sys/prctl.h>
@@ -71,19 +72,16 @@ ipc_unwinder_impl_handle_unwind (IpcUnwinder           *unwinder,
                                  GUnixFDList           *fd_list,
                                  guint                  stack_size,
                                  GVariant              *arg_perf_fds,
-                                 GVariant              *arg_event_fd)
+                                 GVariant              *arg_event_fd,
+                                 GVariant              *arg_capture_fd)
 {
   g_autoptr(GSubprocessLauncher) launcher = NULL;
   g_autoptr(GSubprocess) subprocess = NULL;
-  g_autoptr(GUnixFDList) out_fd_list = NULL;
   g_autoptr(GPtrArray) argv = NULL;
   g_autoptr(GError) error = NULL;
-  g_autofd int our_fd = -1;
+  g_autofd int capture_fd = -1;
-  g_autofd int their_fd = -1;
   g_autofd int event_fd = -1;
   GVariantIter iter;
-  int capture_fd_handle;
-  int pair[2];
   int next_target_fd = 3;
   int perf_fd_handle;
   int cpu;
@@ -116,13 +114,14 @@ ipc_unwinder_impl_handle_unwind (IpcUnwinder           *unwinder,
   g_ptr_array_add (argv, g_strdup (PACKAGE_LIBEXECDIR "/sysprof-live-unwinder"));
   g_ptr_array_add (argv, g_strdup_printf ("--stack-size=%u", stack_size));
 
-  if (-1 == (event_fd = g_unix_fd_list_get (fd_list, g_variant_get_handle (arg_event_fd), &error)))
+  if (-1 == (event_fd = g_unix_fd_list_get (fd_list, g_variant_get_handle (arg_event_fd), &error)) ||
+      -1 == (capture_fd = g_unix_fd_list_get (fd_list, g_variant_get_handle (arg_capture_fd), &error)))
     {
       g_dbus_method_invocation_return_gerror (g_steal_pointer (&invocation), error);
       return TRUE;
     }
 
-  g_ptr_array_add (argv, g_strdup_printf ("--event-fd=%u", next_target_fd));
+  g_ptr_array_add (argv, g_strdup_printf ("--event-fd=%d", next_target_fd));
   g_subprocess_launcher_take_fd (launcher, g_steal_fd (&event_fd), next_target_fd++);
 
   g_variant_iter_init (&iter, arg_perf_fds);
@@ -143,32 +142,8 @@ ipc_unwinder_impl_handle_unwind (IpcUnwinder           *unwinder,
                                      next_target_fd++);
     }
 
-  g_subprocess_launcher_set_child_setup (launcher, child_setup, NULL, NULL);
-
-  if (socketpair (AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, pair) < 0)
-    {
-      int errsv = errno;
-      g_dbus_method_invocation_return_error_literal (g_steal_pointer (&invocation),
-                                                     G_IO_ERROR,
-                                                     g_io_error_from_errno (errsv),
-                                                     g_strerror (errsv));
-      return TRUE;
-    }
-
-  our_fd = g_steal_fd (&pair[0]);
-  their_fd = g_steal_fd (&pair[1]);
-
-  out_fd_list = g_unix_fd_list_new ();
-  capture_fd_handle = g_unix_fd_list_append (out_fd_list, their_fd, &error);
-
-  if (capture_fd_handle < 0)
-    {
-      g_dbus_method_invocation_return_gerror (g_steal_pointer (&invocation), error);
-      return TRUE;
-    }
-
   g_ptr_array_add (argv, g_strdup_printf ("--capture-fd=%d", next_target_fd));
-  g_subprocess_launcher_take_fd (launcher, g_steal_fd (&our_fd), next_target_fd++);
+  g_subprocess_launcher_take_fd (launcher, g_steal_fd (&capture_fd), next_target_fd++);
 
   g_ptr_array_add (argv, NULL);
 
@@ -182,10 +157,7 @@ ipc_unwinder_impl_handle_unwind (IpcUnwinder           *unwinder,
   g_message ("sysprof-live-unwinder started as process %s",
              g_subprocess_get_identifier (subprocess));
 
-  ipc_unwinder_complete_unwind (unwinder,
+  ipc_unwinder_complete_unwind (unwinder, g_steal_pointer (&invocation), NULL);
-                                g_steal_pointer (&invocation),
-                                out_fd_list,
-                                g_variant_new_handle (capture_fd_handle));
 
   g_subprocess_wait_check_async (subprocess,
                                  NULL,

src/sysprofd/org.gnome.Sysprof3.Unwinder.xml

@@ -17,7 +17,7 @@
       <arg type="u" name="stack_size" direction="in"/>
       <arg type="a(hi)" name="perf_fds" direction="in"/>
       <arg type="h" name="event_fd" direction="in"/>
-      <arg type="h" name="capture_fd" direction="out"/>
+      <arg type="h" name="capture_fd" direction="in"/>
     </method>
   </interface>
 </node>