[sysprof/wip/chergert/control-fd] collector: avoid some feedback loops and use socket connections
- From: Christian Hergert <chergert src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [sysprof/wip/chergert/control-fd] collector: avoid some feedback loops and use socket connections
- Date: Tue, 11 Feb 2020 21:39:38 +0000 (UTC)
commit 182bcd5b20e038fe491412106d1ef31f179019c4
Author: Christian Hergert <chergert redhat com>
Date: Tue Feb 11 13:39:23 2020 -0800
collector: avoid some feedback loops and use socket connections
src/libsysprof-capture/sysprof-collector.c | 71 +++++++-----
src/libsysprof/preload/gconstructor.h | 94 +++++++++++++++
src/libsysprof/preload/sysprof-memory-collector.c | 22 +++-
src/libsysprof/sysprof-control-source.c | 134 ++++++++++++++++------
4 files changed, 253 insertions(+), 68 deletions(-)
---
diff --git a/src/libsysprof-capture/sysprof-collector.c b/src/libsysprof-capture/sysprof-collector.c
index 739e219..88acd6d 100644
--- a/src/libsysprof-capture/sysprof-collector.c
+++ b/src/libsysprof-capture/sysprof-collector.c
@@ -122,31 +122,29 @@ request_writer (void)
if (peer_fd > 0)
{
- GInputStream *in_stream;
- GOutputStream *out_stream;
- GIOStream *io_stream;
+ GSocketConnection *stream;
+ GSocket *sock;
g_unix_set_fd_nonblocking (peer_fd, TRUE, NULL);
- in_stream = g_unix_input_stream_new (dup (peer_fd), TRUE);
- out_stream = g_unix_output_stream_new (dup (peer_fd), TRUE);
- io_stream = g_simple_io_stream_new (in_stream, out_stream);
- peer = g_dbus_connection_new_sync (io_stream,
- NULL,
- G_DBUS_CONNECTION_FLAGS_DELAY_MESSAGE_PROCESSING,
- NULL,
- NULL,
- NULL);
-
- if (peer != NULL)
+ if ((sock = g_socket_new_from_fd (peer_fd, NULL)))
{
- g_dbus_connection_set_exit_on_close (peer, FALSE);
- g_dbus_connection_start_message_processing (peer);
+ stream = g_socket_connection_factory_create_connection (sock);
+ g_printerr ("COLLECTOR TYPE: %s\n", G_OBJECT_TYPE_NAME (stream));
+ peer = g_dbus_connection_new_sync (G_IO_STREAM (stream), NULL,
+ (G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT |
+ G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS |
+ G_DBUS_CONNECTION_FLAGS_DELAY_MESSAGE_PROCESSING),
+ NULL, NULL, NULL);
+ if (peer != NULL)
+ {
+ g_dbus_connection_set_exit_on_close (peer, FALSE);
+ g_dbus_connection_start_message_processing (peer);
+ }
+
+ g_clear_object (&stream);
+ g_clear_object (&sock);
}
-
- g_clear_object (&in_stream);
- g_clear_object (&out_stream);
- g_clear_object (&io_stream);
}
}
@@ -154,6 +152,7 @@ request_writer (void)
{
GUnixFDList *out_fd_list = NULL;
GVariant *reply;
+ GError *error = NULL;
reply = g_dbus_connection_call_with_unix_fd_list_sync (peer,
NULL,
@@ -163,21 +162,37 @@ request_writer (void)
g_variant_new ("()"),
G_VARIANT_TYPE ("(h)"),
G_DBUS_CALL_FLAGS_NONE,
- -1,
+ 1000, /* 1 second */
NULL,
&out_fd_list,
NULL,
- NULL);
+ &error);
+
+ if (error != NULL)
+ {
+ g_printerr ("ERROR: %s\n", error->message);
+ g_error_free (error);
+ }
if (reply != NULL)
{
- int fd = g_unix_fd_list_get (out_fd_list, 0, NULL);
+ int handle = -1;
+ int fd;
+
+ g_variant_get (reply, "(h)", &handle, NULL);
- if (fd > -1)
- writer = sysprof_capture_writer_new_from_fd (fd, 0);
+ if (handle > -1)
+ {
+ fd = g_unix_fd_list_get (out_fd_list, handle, NULL);
+
+ if (fd > -1)
+ writer = sysprof_capture_writer_new_from_fd (fd, 0);
+ }
g_variant_unref (reply);
}
+
+ g_printerr ("Writer: %p\n", writer);
}
return g_steal_pointer (&writer);
@@ -204,12 +219,12 @@ sysprof_collector_get (void)
/* We might have gotten here recursively */
if G_UNLIKELY (collector == COLLECTOR_MAGIC_CREATING)
- return NULL;
+ return COLLECTOR_MAGIC_CREATING;
if G_LIKELY (collector != NULL)
return collector;
- if (use_single_trace () && shared_collector != NULL)
+ if (use_single_trace () && shared_collector != COLLECTOR_MAGIC_CREATING)
return shared_collector;
{
@@ -284,7 +299,7 @@ sysprof_collector_init (void)
#define ADD_TO_COLLECTOR(func) \
G_STMT_START { \
const SysprofCollector *collector = sysprof_collector_get (); \
- if (collector != NULL) \
+ if (collector != COLLECTOR_MAGIC_CREATING) \
{ \
if (collector->is_shared) { G_LOCK (control_fd); } \
if (collector->writer != NULL) { func; } \
diff --git a/src/libsysprof/preload/gconstructor.h b/src/libsysprof/preload/gconstructor.h
new file mode 100644
index 0000000..df98f83
--- /dev/null
+++ b/src/libsysprof/preload/gconstructor.h
@@ -0,0 +1,94 @@
+/*
+ If G_HAS_CONSTRUCTORS is true then the compiler support *both* constructors and
+ destructors, in a sane way, including e.g. on library unload. If not you're on
+ your own.
+
+ Some compilers need #pragma to handle this, which does not work with macros,
+ so the way you need to use this is (for constructors):
+
+ #ifdef G_DEFINE_CONSTRUCTOR_NEEDS_PRAGMA
+ #pragma G_DEFINE_CONSTRUCTOR_PRAGMA_ARGS(my_constructor)
+ #endif
+ G_DEFINE_CONSTRUCTOR(my_constructor)
+ static void my_constructor(void) {
+ ...
+ }
+
+*/
+
+#ifndef __GTK_DOC_IGNORE__
+
+#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 7)
+
+#define G_HAS_CONSTRUCTORS 1
+
+#define G_DEFINE_CONSTRUCTOR(_func) static void __attribute__((constructor)) _func (void);
+#define G_DEFINE_DESTRUCTOR(_func) static void __attribute__((destructor)) _func (void);
+
+#elif defined (_MSC_VER) && (_MSC_VER >= 1500)
+/* Visual studio 2008 and later has _Pragma */
+
+#define G_HAS_CONSTRUCTORS 1
+
+#define G_DEFINE_CONSTRUCTOR(_func) \
+ static void _func(void); \
+ static int _func ## _wrapper(void) { _func(); return 0; } \
+ __pragma(section(".CRT$XCU",read)) \
+ __declspec(allocate(".CRT$XCU")) static int (* _array ## _func)(void) = _func ## _wrapper;
+
+#define G_DEFINE_DESTRUCTOR(_func) \
+ static void _func(void); \
+ static int _func ## _constructor(void) { atexit (_func); return 0; } \
+ __pragma(section(".CRT$XCU",read)) \
+ __declspec(allocate(".CRT$XCU")) static int (* _array ## _func)(void) = _func ## _constructor;
+
+#elif defined (_MSC_VER)
+
+#define G_HAS_CONSTRUCTORS 1
+
+/* Pre Visual studio 2008 must use #pragma section */
+#define G_DEFINE_CONSTRUCTOR_NEEDS_PRAGMA 1
+#define G_DEFINE_DESTRUCTOR_NEEDS_PRAGMA 1
+
+#define G_DEFINE_CONSTRUCTOR_PRAGMA_ARGS(_func) \
+ section(".CRT$XCU",read)
+#define G_DEFINE_CONSTRUCTOR(_func) \
+ static void _func(void); \
+ static int _func ## _wrapper(void) { _func(); return 0; } \
+ __declspec(allocate(".CRT$XCU")) static int (*p)(void) = _func ## _wrapper;
+
+#define G_DEFINE_DESTRUCTOR_PRAGMA_ARGS(_func) \
+ section(".CRT$XCU",read)
+#define G_DEFINE_DESTRUCTOR(_func) \
+ static void _func(void); \
+ static int _func ## _constructor(void) { atexit (_func); return 0; } \
+ __declspec(allocate(".CRT$XCU")) static int (* _array ## _func)(void) = _func ## _constructor;
+
+#elif defined(__SUNPRO_C)
+
+/* This is not tested, but i believe it should work, based on:
+ * http://opensource.apple.com/source/OpenSSL098/OpenSSL098-35/src/fips/fips_premain.c
+ */
+
+#define G_HAS_CONSTRUCTORS 1
+
+#define G_DEFINE_CONSTRUCTOR_NEEDS_PRAGMA 1
+#define G_DEFINE_DESTRUCTOR_NEEDS_PRAGMA 1
+
+#define G_DEFINE_CONSTRUCTOR_PRAGMA_ARGS(_func) \
+ init(_func)
+#define G_DEFINE_CONSTRUCTOR(_func) \
+ static void _func(void);
+
+#define G_DEFINE_DESTRUCTOR_PRAGMA_ARGS(_func) \
+ fini(_func)
+#define G_DEFINE_DESTRUCTOR(_func) \
+ static void _func(void);
+
+#else
+
+/* constructors not supported for this compiler */
+
+#endif
+
+#endif /* __GTK_DOC_IGNORE__ */
diff --git a/src/libsysprof/preload/sysprof-memory-collector.c
b/src/libsysprof/preload/sysprof-memory-collector.c
index 61295ea..8187ffe 100644
--- a/src/libsysprof/preload/sysprof-memory-collector.c
+++ b/src/libsysprof/preload/sysprof-memory-collector.c
@@ -16,6 +16,8 @@
#include <sysprof-capture.h>
#include <unistd.h>
+#include "gconstructor.h"
+
typedef void *(* RealMalloc) (size_t);
typedef void (* RealFree) (void *);
typedef void *(* RealCalloc) (size_t, size_t);
@@ -47,6 +49,22 @@ static RealAlignedAlloc real_aligned_alloc;
static RealPosixMemalign real_posix_memalign;
static RealMemalign real_memalign;
+#if defined (G_HAS_CONSTRUCTORS)
+# ifdef G_DEFINE_CONSTRUCTOR_NEEDS_PRAGMA
+# pragma G_DEFINE_CONSTRUCTOR_PRAGMA_ARGS(collector_init_ctor)
+# endif
+G_DEFINE_CONSTRUCTOR(collector_init_ctor)
+#else
+# error Your platform/compiler is missing constructor support
+#endif
+
+static void
+collector_init_ctor (void)
+{
+ sysprof_collector_init ();
+ g_atomic_int_set (&collector_ready, TRUE);
+}
+
static guint
backtrace_func (SysprofCaptureAddress *addrs,
guint n_addrs,
@@ -149,10 +167,6 @@ hook_memtable (void)
real_memalign = dlsym (RTLD_NEXT, "memalign");
unsetenv ("LD_PRELOAD");
-
- sysprof_collector_init ();
-
- g_atomic_int_set (&collector_ready, TRUE);
}
static inline void
diff --git a/src/libsysprof/sysprof-control-source.c b/src/libsysprof/sysprof-control-source.c
index 27aa980..edf1696 100644
--- a/src/libsysprof/sysprof-control-source.c
+++ b/src/libsysprof/sysprof-control-source.c
@@ -87,47 +87,115 @@ on_handle_create_writer_cb (IpcCollector *collector,
SysprofControlSource *self)
{
gchar writer_tmpl[] = "sysprof-collector-XXXXXX";
+ g_autoptr(GError) error = NULL;
int fd;
g_assert (IPC_IS_COLLECTOR (collector));
g_assert (G_IS_DBUS_METHOD_INVOCATION (invocation));
g_assert (SYSPROF_IS_CONTROL_SOURCE (self));
- fd = g_mkstemp_full (writer_tmpl, O_CLOEXEC, 0);
+ g_printerr ("Request for new writer\n");
+
+ fd = g_mkstemp_full (writer_tmpl, O_RDWR | O_CLOEXEC, 0640);
if (fd > -1)
{
- g_autoptr(GUnixFDList) out_fd_list = g_unix_fd_list_new_from_array (&fd, 1);
+ g_autoptr(GUnixFDList) out_fd_list = g_unix_fd_list_new ();
+ gint handle;
+
+ handle = g_unix_fd_list_append (out_fd_list, fd, &error);
+ close (fd);
- if (out_fd_list != NULL)
+ if (handle > -1)
{
+ g_print ("New fd list with reply %d (fd=%d) %s %p\n", handle, fd, writer_tmpl, out_fd_list);
+
g_ptr_array_add (self->files, g_strdup (writer_tmpl));
ipc_collector_complete_create_writer (collector,
g_steal_pointer (&invocation),
out_fd_list,
- g_variant_new_handle (0));
+ g_variant_new_handle (handle));
+
+ g_printerr ("Sent\n");
+
return TRUE;
}
}
- g_dbus_method_invocation_return_error (g_steal_pointer (&invocation),
- G_DBUS_ERROR,
- G_DBUS_ERROR_FAILED,
- "Failed to create FD for writer");
+ g_printerr ("Womp sending failure\n");
+
+ if (error != NULL)
+ g_dbus_method_invocation_return_gerror (g_steal_pointer (&invocation), error);
+ else
+ g_dbus_method_invocation_return_error (g_steal_pointer (&invocation),
+ G_DBUS_ERROR,
+ G_DBUS_ERROR_FAILED,
+ "Failed to create FD for writer");
return TRUE;
}
+static void
+on_bus_closed_cb (GDBusConnection *connection,
+ gboolean remote_peer_vanished,
+ const GError *error,
+ SysprofControlSource *self)
+{
+ g_printerr ("Bus connection closed: %s\n", error->message);
+}
+
+static void
+new_connection_cb (GObject *object,
+ GAsyncResult *result,
+ gpointer user_data)
+{
+ g_autoptr(GDBusConnection) conn = NULL;
+ g_autoptr(SysprofControlSource) self = user_data;
+ g_autoptr(GError) error = NULL;
+
+ g_assert (G_IS_ASYNC_RESULT (result));
+ g_assert (SYSPROF_IS_CONTROL_SOURCE (self));
+
+ if (!(conn = g_dbus_connection_new_finish (result, &error)))
+ {
+ sysprof_source_emit_failed (SYSPROF_SOURCE (self), error);
+ return;
+ }
+
+ g_set_object (&self->conn, conn);
+
+ self->collector = ipc_collector_skeleton_new ();
+
+ g_signal_connect_object (conn,
+ "closed",
+ G_CALLBACK (on_bus_closed_cb),
+ self,
+ 0);
+
+ g_signal_connect_object (self->collector,
+ "handle-create-writer",
+ G_CALLBACK (on_handle_create_writer_cb),
+ self,
+ 0);
+
+ g_dbus_interface_skeleton_export (G_DBUS_INTERFACE_SKELETON (self->collector),
+ conn,
+ "/org/gnome/Sysprof3/Collector",
+ NULL);
+
+ g_dbus_connection_start_message_processing (conn);
+}
+
static void
sysprof_control_source_modify_spawn (SysprofSource *source,
SysprofSpawnable *spawnable)
{
SysprofControlSource *self = (SysprofControlSource *)source;
g_autofree gchar *child_no_str = NULL;
- g_autoptr(GInputStream) in_stream = NULL;
- g_autoptr(GOutputStream) out_stream = NULL;
- g_autoptr(GIOStream) io_stream = NULL;
+ g_autoptr(GSocketConnection) stream = NULL;
g_autoptr(GDBusConnection) conn = NULL;
+ g_autoptr(GSocket) sock = NULL;
+ g_autofree gchar *guid = g_dbus_generate_guid ();
int fds[2];
int child_no;
@@ -135,7 +203,7 @@ sysprof_control_source_modify_spawn (SysprofSource *source,
g_assert (SYSPROF_IS_SPAWNABLE (spawnable));
/* Create a socket pair to communicate D-Bus protocol over */
- if (socketpair (PF_LOCAL, SOCK_STREAM, 0, fds) != 0)
+ if (socketpair (AF_LOCAL, SOCK_STREAM, 0, fds) != 0)
return;
g_unix_set_fd_nonblocking (fds[0], TRUE, NULL);
@@ -148,32 +216,26 @@ sysprof_control_source_modify_spawn (SysprofSource *source,
child_no_str = g_strdup_printf ("%d", child_no);
sysprof_spawnable_setenv (spawnable, "SYSPROF_CONTROL_FD", child_no_str);
- /* Create our D-Bus connection for our side and export our service
- * that can create new writers.
+ /* We need an IOStream for GDBusConnection to use. Since we need
+ * the ability to pass FDs, it must be a GUnixSocketConnection.
*/
- in_stream = g_unix_input_stream_new (dup (fds[0]), TRUE);
- out_stream = g_unix_output_stream_new (fds[0], TRUE);
- io_stream = g_simple_io_stream_new (in_stream, out_stream);
- conn = g_dbus_connection_new_sync (io_stream, NULL,
- G_DBUS_CONNECTION_FLAGS_DELAY_MESSAGE_PROCESSING,
- NULL, NULL, NULL);
-
- g_set_object (&self->conn, conn);
-
- self->collector = ipc_collector_skeleton_new ();
-
- g_signal_connect_object (self->collector,
- "handle-create-writer",
- G_CALLBACK (on_handle_create_writer_cb),
- self,
- 0);
-
- g_dbus_interface_skeleton_export (G_DBUS_INTERFACE_SKELETON (self->collector),
- conn,
- "/org/gnome/Sysprof3/Collector",
- NULL);
+ if (!(sock = g_socket_new_from_fd (fds[0], NULL)))
+ {
+ close (fds[0]);
+ g_critical ("Failed to create GSocket");
+ return;
+ }
- g_dbus_connection_start_message_processing (conn);
+ stream = g_socket_connection_factory_create_connection (sock);
+ g_dbus_connection_new (G_IO_STREAM (stream),
+ guid,
+ (G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_SERVER |
+ G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS |
+ G_DBUS_CONNECTION_FLAGS_DELAY_MESSAGE_PROCESSING),
+ NULL,
+ NULL,
+ new_connection_cb,
+ g_object_ref (self));
}
static void
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
