libdlog: do lazy initialisation (no constructor)

Reasons: some long-living processes which start before dlog can initialize
properly but don't log anything until later, the ability to wrap some calls
before dlog can use them. See the comments inside for more details.

Change-Id: I808a2e7774f74a4c7ead68c6a6dac7cc7d2082e8
Signed-off-by: Michal Bloch <m.bloch@samsung.com>

diff --git a/src/libdlog/log.c b/src/libdlog/log.c
index ad785e5..c1906b1 100644 (file)
--- a/src/libdlog/log.c
+++ b/src/libdlog/log.c
@@ -53,6 +53,8 @@ void (*destroy_backend)();
 
 pthread_rwlock_t log_limiter_lock = PTHREAD_RWLOCK_INITIALIZER;
 static pthread_rwlock_t log_destruction_lock = PTHREAD_RWLOCK_INITIALIZER;
+static pthread_mutex_t log_construction_lock = PTHREAD_MUTEX_INITIALIZER;
+static bool is_initialized = false;
 
 extern void __dlog_init_pipe(const struct log_config *conf);
 extern void __dlog_init_android(const struct log_config *conf);
@@ -143,16 +145,27 @@ void __update_plog(const struct log_config *conf)
 /**
  * @brief Configure the library
  * @details Reads relevant config values
- * @remarks The constructor has the highest possible priority (100 and below are reserved).
- *          This is because other libraries are likely to use libdlog and might want to
- *          use it as early as in their own constructor. This means that libdlog must be
- *          constructed as soon as possible.
+ * @remarks This is more or less a constructor, but there are some obstacles
+ *          to using it as such (i.e. with attribute constructor):
+ *
+ *  - some important pieces of the system link to dlog, they start very early
+ *    such that dlog can't properly initialize (which lasts for program lifetime)
+ *    but don't actually log anything until later on and would be fine under lazy
+ *    initialisation. The way to do it "properly" would be to expose this function
+ *    into the API so that people can manually call it when they're ready, but
+ *    one of the design goals of the current API is that it requires absolutely no
+ *    other calls than `dlog_print`. Changing it would require somebody with a
+ *    bird's eye view of the system to produce a design so I wouldn't count on it.
+ *
+ *  - the constructor would need to have as high of a priority as possible (so as
+ *    to minimize the risk of another library's constructor using uninitialized data)
+ *    but at the same time others might want some room to wrap functions before
+ *    dlog uses them (think mprobe/mcheck). This would also require a design pass.
  */
-#ifdef UNIT_TEST
-void __configure(void)
-#else
-static void __attribute__((constructor(101))) __configure(void)
+#ifndef UNIT_TEST
+static
 #endif
+void __configure(void)
 {
        struct log_config config;
 
@@ -173,6 +186,43 @@ out:
        return;
 }
 
+static bool first = true;
+static void initialize()
+{
+       if (is_initialized)
+               return;
+
+       /* The mutex acts as a barrier, but otherwise the C language's
+        * machine abstraction is single-threaded. This means that the
+        * compiler is free to rearrange calls inside the mutex according
+        * to the as-if rule because it doesn't care if another thread can
+        * access it in parallel. In particular, `is_initialized = true`
+        * directly after `__configure()` could be rearranged to go in
+        * front of it because it is not touched inside that function
+        * if the compiler thinks it helps somehow (not unlikely: since
+        * it is checked before the mutex, it is very probable for it to
+        * still be in the CPU register or something like that). On top
+        * of that, some architectures (in particular, armv7l) don't have
+        * strict memory guarantees and can reorder actual memory stores
+        * on their own, even if the compiler didn't do anything fancy
+        * when creating machine code. For more info about the issue,
+        * see https://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf
+        *
+        * Ultimately this means that there needs to be some sort of
+        * barrier between `__configure` and `is_initialized = true`,
+        * and the simplest way to achieve that is to just wait until
+        * the second entry into the mutex. */
+
+       pthread_mutex_lock(&log_construction_lock);
+               if (first) {
+                       __configure();
+                       first = false; // pop this cherry
+               } else {
+                       is_initialized = true;
+               }
+       pthread_mutex_unlock(&log_construction_lock);
+}
+
 /**
  * @brief Fatal assertion
  * @details Conditionally crash the sucka who sent the log
@@ -264,6 +314,7 @@ static int __write_to_log(log_id_t log_id, int prio, const char *tag, const char
         * but giving this guarantee makes everything a lot simpler as it removes
         * the risk of something suddenly becoming NULL during processing. */
        pthread_rwlock_rdlock(&log_destruction_lock);
+       initialize();
        ret = write_to_log ? __write_to_log_critical_section(log_id, prio, tag, fmt, ap, check_should_log) : DLOG_ERROR_NOT_PERMITTED;
        pthread_rwlock_unlock(&log_destruction_lock);
 
@@ -339,6 +390,8 @@ void __dlog_fini(void)
                destroy_backend = NULL;
        }
        write_to_log = NULL;
+       is_initialized = false;
+       first = true;
 
        __log_limiter_destroy();
        __dynamic_config_destroy();