From 9298df725192e8240227ea66abd68b8bcd508067 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Wed, 13 Nov 2024 15:40:52 -0800 Subject: [PATCH 27/33] 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 | 65 ++++++++++++-------- src/sysprofd/ipc-unwinder-impl.c | 46 +++----------- src/sysprofd/org.gnome.Sysprof3.Unwinder.xml | 2 +- 3 files changed, 48 insertions(+), 65 deletions(-) diff --git a/src/libsysprof/sysprof-user-sampler.c b/src/libsysprof/sysprof-user-sampler.c index 44b4d318..6079708e 100644 --- a/src/libsysprof/sysprof-user-sampler.c +++ b/src/libsysprof/sysprof-user-sampler.c @@ -20,6 +20,8 @@ #include "config.h" +#include + #include #include @@ -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) && - -1 != (capture_fd = g_unix_fd_list_get (out_fd_list, g_variant_get_handle (out_capture_fd), &error))) - promise_resolve_fd (promise, g_steal_fd (&capture_fd)); + if (ipc_unwinder_call_unwind_finish (IPC_UNWINDER (object), &out_fd_list, result, &error)) + dex_promise_resolve_boolean (promise, TRUE); 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 (fd == -1) + if (!dex_await (dex_ref (promise), &error)) { _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"); - muxer_source = sysprof_muxer_source_new (g_steal_fd (&record->sampler->capture_fd), writer); - g_source_set_static_name (muxer_source, "[stack-muxer]"); - g_source_attach (muxer_source, NULL); + 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_read), writer); + g_source_set_static_name (muxer_source, "[stack-muxer]"); + g_source_attach (muxer_source, NULL); - if (!dex_await (dex_ref (record->cancellable), &error)) - g_debug ("UserSampler shutting down for reason: %s", error->message); + if (!dex_await (dex_ref (record->cancellable), &error)) + 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); } diff --git a/src/sysprofd/ipc-unwinder-impl.c b/src/sysprofd/ipc-unwinder-impl.c index 4341516b..e6a0d7ab 100644 --- a/src/sysprofd/ipc-unwinder-impl.c +++ b/src/sysprofd/ipc-unwinder-impl.c @@ -24,6 +24,7 @@ #include "config.h" #include +#include #include #include @@ -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 their_fd = -1; + g_autofd int capture_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, - g_steal_pointer (&invocation), - out_fd_list, - g_variant_new_handle (capture_fd_handle)); + ipc_unwinder_complete_unwind (unwinder, g_steal_pointer (&invocation), NULL); g_subprocess_wait_check_async (subprocess, NULL, diff --git a/src/sysprofd/org.gnome.Sysprof3.Unwinder.xml b/src/sysprofd/org.gnome.Sysprof3.Unwinder.xml index fb2c7848..86b3bdbe 100644 --- a/src/sysprofd/org.gnome.Sysprof3.Unwinder.xml +++ b/src/sysprofd/org.gnome.Sysprof3.Unwinder.xml @@ -17,7 +17,7 @@ - + -- 2.45.2