[sysprof/wip/chergert/control-fd: 11/11] capture: progress on recursive init from collector

[Christian Hergert <chergert src gnome org>]

commit cbf3cbca64b72d5c28aac9386d14c83a4e0841e8
Author: Christian Hergert <chergert redhat com>
Date:   Tue Feb 11 11:23:14 2020 -0800

    capture: progress on recursive init from collector
    
    And use it from the preload for memory profiling.

 src/libsysprof-capture/sysprof-collector.c        | 35 +++++++++--
 src/libsysprof-capture/sysprof-collector.h        |  2 +
 src/libsysprof/preload/sysprof-memory-collector.c | 77 ++++-------------------
 src/libsysprof/sysprof-control-source.c           | 17 +++--
 4 files changed, 56 insertions(+), 75 deletions(-)
---
diff --git a/src/libsysprof-capture/sysprof-collector.c b/src/libsysprof-capture/sysprof-collector.c
index dd2c64c..739e219 100644
--- a/src/libsysprof-capture/sysprof-collector.c
+++ b/src/libsysprof-capture/sysprof-collector.c
@@ -89,6 +89,8 @@ typedef struct
 # define sysprof_current_cpu (-1)
 #endif
 
+#define COLLECTOR_MAGIC_CREATING GSIZE_TO_POINTER(0xE0E0E0E1)
+
 static SysprofCaptureWriter   *request_writer         (void);
 static void                    sysprof_collector_free (gpointer data);
 static const SysprofCollector *sysprof_collector_get  (void);
@@ -186,12 +188,12 @@ sysprof_collector_free (gpointer data)
 {
   SysprofCollector *collector = data;
 
-  if (collector != NULL)
+  if (collector != NULL && collector != COLLECTOR_MAGIC_CREATING)
     {
       if (collector->writer != NULL)
         sysprof_capture_writer_flush (collector->writer);
       g_clear_pointer (&collector->writer, sysprof_capture_writer_unref);
-      g_slice_free (SysprofCollector, collector);
+      g_free (collector);
     }
 }
 
@@ -200,6 +202,10 @@ sysprof_collector_get (void)
 {
   const SysprofCollector *collector = g_private_get (&collector_key);
 
+  /* We might have gotten here recursively */
+  if G_UNLIKELY (collector == COLLECTOR_MAGIC_CREATING)
+    return NULL;
+
   if G_LIKELY (collector != NULL)
     return collector;
 
@@ -211,7 +217,9 @@ sysprof_collector_get (void)
 
     G_LOCK (control_fd);
 
-    self = g_slice_new0 (SysprofCollector);
+    g_private_replace (&collector_key, COLLECTOR_MAGIC_CREATING);
+
+    self = g_new0 (SysprofCollector, 1);
     self->pid = getpid ();
 #ifdef __linux__
     self->tid = syscall (__NR_gettid, 0);
@@ -261,12 +269,27 @@ sysprof_collector_get (void)
   }
 }
 
+void
+sysprof_collector_init (void)
+{
+  static gsize once_init;
+
+  if (g_once_init_enter (&once_init))
+    {
+      (void)sysprof_collector_get ();
+      g_once_init_leave (&once_init, TRUE);
+    }
+}
+
 #define ADD_TO_COLLECTOR(func)                                    \
   G_STMT_START {                                                  \
     const SysprofCollector *collector = sysprof_collector_get (); \
-    if (collector->is_shared) { G_LOCK (control_fd); }            \
-    if (collector->writer != NULL) { func; }                      \
-    if (collector->is_shared) { G_UNLOCK (control_fd); }          \
+    if (collector != NULL)                                        \
+      {                                                           \
+        if (collector->is_shared) { G_LOCK (control_fd); }        \
+        if (collector->writer != NULL) { func; }                  \
+        if (collector->is_shared) { G_UNLOCK (control_fd); }      \
+      }                                                           \
   } G_STMT_END
 
 void
diff --git a/src/libsysprof-capture/sysprof-collector.h b/src/libsysprof-capture/sysprof-collector.h
index ca7b1d5..bc23fb0 100644
--- a/src/libsysprof-capture/sysprof-collector.h
+++ b/src/libsysprof-capture/sysprof-collector.h
@@ -61,6 +61,8 @@
 
 G_BEGIN_DECLS
 
+SYSPROF_AVAILABLE_IN_3_36
+void                  sysprof_collector_init             (void);
 SYSPROF_AVAILABLE_IN_3_36
 void                  sysprof_collector_embed_file       (const gchar                      *path,
                                                           const guint8                     *data,
diff --git a/src/libsysprof/preload/sysprof-memory-collector.c 
b/src/libsysprof/preload/sysprof-memory-collector.c
index 1e6b240..61295ea 100644
--- a/src/libsysprof/preload/sysprof-memory-collector.c
+++ b/src/libsysprof/preload/sysprof-memory-collector.c
@@ -36,10 +36,8 @@ static void *scratch_realloc (void *, size_t);
 static void *scratch_calloc  (size_t, size_t);
 static void  scratch_free    (void *);
 
-static G_LOCK_DEFINE (writer);
-static SysprofCaptureWriter *writer;
+static int collector_ready;
 static int hooked;
-static int pid;
 static ScratchAlloc scratch;
 static RealCalloc real_calloc = scratch_calloc;
 static RealFree real_free = scratch_free;
@@ -134,19 +132,9 @@ scratch_free (void *ptr)
     return;
 }
 
-static void
-flush_writer (void)
-{
-  G_LOCK (writer);
-  sysprof_capture_writer_flush (writer);
-  G_UNLOCK (writer);
-}
-
 static void
 hook_memtable (void)
 {
-  const gchar *env;
-
   if (hooked)
     return;
 
@@ -162,69 +150,30 @@ hook_memtable (void)
 
   unsetenv ("LD_PRELOAD");
 
-  pid = getpid ();
-
-  /* TODO: We want an API that let's us create a new writer
-   * per-thread instead of something like this (or using an
-   * environment variable). That will require a control channel
-   * to sysprof to request new writer/muxed APIs.
-   */
-
-  env = getenv ("MEMPROF_TRACE_FD");
-
-  if (env != NULL)
-    {
-      int fd = atoi (env);
-
-      if (fd > 0)
-        writer = sysprof_capture_writer_new_from_fd (fd, 0);
-    }
+  sysprof_collector_init ();
 
-  if (writer == NULL)
-    writer = sysprof_capture_writer_new ("memory.syscap", 0);
-
-  atexit (flush_writer);
+  g_atomic_int_set (&collector_ready, TRUE);
 }
 
-#define gettid() syscall(__NR_gettid, 0)
-
 static inline void
 track_malloc (void   *ptr,
               size_t  size)
 {
-  if G_UNLIKELY (!writer)
-    return;
-
-  G_LOCK (writer);
-  sysprof_capture_writer_add_allocation (writer,
-                                         SYSPROF_CAPTURE_CURRENT_TIME,
-                                         sched_getcpu (),
-                                         pid,
-                                         gettid(),
-                                         GPOINTER_TO_SIZE (ptr),
-                                         size,
-                                         backtrace_func,
-                                         NULL);
-  G_UNLOCK (writer);
+  if G_LIKELY (collector_ready != FALSE)
+    sysprof_collector_allocate (GPOINTER_TO_SIZE (ptr),
+                                size,
+                                backtrace_func,
+                                NULL);
 }
 
 static inline void
 track_free (void *ptr)
 {
-  if G_UNLIKELY (!writer)
-    return;
-
-  G_LOCK (writer);
-  sysprof_capture_writer_add_allocation (writer,
-                                         SYSPROF_CAPTURE_CURRENT_TIME,
-                                         sched_getcpu (),
-                                         pid,
-                                         gettid(),
-                                         GPOINTER_TO_SIZE (ptr),
-                                         0,
-                                         backtrace_func,
-                                         0);
-  G_UNLOCK (writer);
+  if G_LIKELY (collector_ready != FALSE)
+    sysprof_collector_allocate (GPOINTER_TO_SIZE (ptr),
+                                0,
+                                backtrace_func,
+                                NULL);
 }
 
 void *
diff --git a/src/libsysprof/sysprof-control-source.c b/src/libsysprof/sysprof-control-source.c
index 40193e8..b253bc2 100644
--- a/src/libsysprof/sysprof-control-source.c
+++ b/src/libsysprof/sysprof-control-source.c
@@ -23,6 +23,7 @@
 #include "config.h"
 
 #include <fcntl.h>
+#include <glib-unix.h>
 #include <gio/gunixfdlist.h>
 #include <gio/gunixinputstream.h>
 #include <gio/gunixoutputstream.h>
@@ -139,6 +140,9 @@ sysprof_control_source_modify_spawn (SysprofSource    *source,
   if (socketpair (PF_LOCAL, SOCK_STREAM, 0, fds) != 0)
     return;
 
+  g_unix_set_fd_nonblocking (fds[0], TRUE, NULL);
+  g_unix_set_fd_nonblocking (fds[1], TRUE, NULL);
+
   /* @child_no is assigned the FD the child will receive. We can
    * use that to set the environment vaiable of the control FD.
    */
@@ -218,13 +222,16 @@ sysprof_control_source_supplement (SysprofSource        *source,
       const gchar *filename = g_ptr_array_index (self->files, i);
       int fd = open (filename, O_RDONLY);
 
-      /* TODO: We can't simply splice these until we've forced the process
-       * to flush the buffers (unless they've already exited).
-       */
-
       if (fd > -1)
         {
-          _sysprof_capture_writer_splice_from_fd (self->writer, fd, NULL);
+          SysprofCaptureReader *worker = sysprof_capture_reader_new_from_fd (fd, NULL);
+
+          if (reader != NULL)
+            {
+              sysprof_capture_writer_cat (self->writer, worker, NULL);
+              sysprof_capture_reader_unref (worker);
+            }
+
           close (fd);
         }
     }