Don't abort when failed to write tracing message.

Also make the code thread-safe with lock.

Bug: 20666100
Change-Id: I0f331a617b75280f36179c187418450230d713ef
(cherry picked from commit 166112531558a1d4ea179c29147f27db7045db22)

1 files changed, 47 insertions, 38 deletions

diff --git a/libc/bionic/bionic_systrace.cpp b/libc/bionic/bionic_systrace.cpp
index 10521cf..103aa8f 100644
--- a/libc/bionic/bionic_systrace.cpp
+++ b/libc/bionic/bionic_systrace.cpp
@@ -21,6 +21,7 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "private/bionic_lock.h"
 #include "private/bionic_systrace.h"
 #include "private/libc_logging.h"
 
@@ -29,12 +30,17 @@
 
 #define WRITE_OFFSET   32
 
-static const prop_info* g_pinfo = NULL;
+constexpr char SYSTRACE_PROPERTY_NAME[] = "debug.atrace.tags.enableflags";
+
+static Lock g_lock;
+static const prop_info* g_pinfo;
 static uint32_t g_serial = -1;
-static uint64_t g_tags = 0;
+static uint64_t g_tags;
 static int g_trace_marker_fd = -1;
 
 static bool should_trace() {
+  bool result = false;
+  g_lock.lock();
   // If g_pinfo is null, this means that systrace hasn't been run and it's safe to
   // assume that no trace writing will need to take place.  However, to avoid running
   // this costly find check each time, we set it to a non-tracing value so that next
@@ -42,32 +48,39 @@ static bool should_trace() {
   // this function also deals with the bootup case, during which the call to property
   // set will fail if the property server hasn't yet started.
   if (g_pinfo == NULL) {
-    g_pinfo = __system_property_find("debug.atrace.tags.enableflags");
+    g_pinfo = __system_property_find(SYSTRACE_PROPERTY_NAME);
     if (g_pinfo == NULL) {
-      __system_property_set("debug.atrace.tags.enableflags", "0");
-      g_pinfo = __system_property_find("debug.atrace.tags.enableflags");
-      if (g_pinfo == NULL) {
-        return false;
-      }
+      __system_property_set(SYSTRACE_PROPERTY_NAME, "0");
+      g_pinfo = __system_property_find(SYSTRACE_PROPERTY_NAME);
     }
   }
-
-  // Find out which tags have been enabled on the command line and set
-  // the value of tags accordingly.  If the value of the property changes,
-  // the serial will also change, so the costly system_property_read function
-  // can be avoided by calling the much cheaper system_property_serial
-  // first.  The values within pinfo may change, but its location is guaranteed
-  // not to move.
-  const uint32_t cur_serial = __system_property_serial(g_pinfo);
-  if (cur_serial != g_serial) {
-    g_serial = cur_serial;
-    char value[PROP_VALUE_MAX];
-    __system_property_read(g_pinfo, 0, value);
-    g_tags = strtoull(value, NULL, 0);
+  if (g_pinfo != NULL) {
+    // Find out which tags have been enabled on the command line and set
+    // the value of tags accordingly.  If the value of the property changes,
+    // the serial will also change, so the costly system_property_read function
+    // can be avoided by calling the much cheaper system_property_serial
+    // first.  The values within pinfo may change, but its location is guaranteed
+    // not to move.
+    uint32_t cur_serial = __system_property_serial(g_pinfo);
+    if (cur_serial != g_serial) {
+      g_serial = cur_serial;
+      char value[PROP_VALUE_MAX];
+      __system_property_read(g_pinfo, 0, value);
+      g_tags = strtoull(value, NULL, 0);
+    }
+    result = ((g_tags & ATRACE_TAG_BIONIC) != 0);
   }
+  g_lock.unlock();
+  return result;
+}
 
-  // Finally, verify that this tag value enables bionic tracing.
-  return ((g_tags & ATRACE_TAG_BIONIC) != 0);
+static int get_trace_marker_fd() {
+  g_lock.lock();
+  if (g_trace_marker_fd == -1) {
+    g_trace_marker_fd = open("/sys/kernel/debug/tracing/trace_marker", O_CLOEXEC | O_WRONLY);
+  }
+  g_lock.unlock();
+  return g_trace_marker_fd;
 }
 
 ScopedTrace::ScopedTrace(const char* message) {
@@ -75,11 +88,9 @@ ScopedTrace::ScopedTrace(const char* message) {
     return;
   }
 
-  if (g_trace_marker_fd == -1) {
-    g_trace_marker_fd = open("/sys/kernel/debug/tracing/trace_marker", O_CLOEXEC | O_WRONLY);
-    if (g_trace_marker_fd == -1) {
-      __libc_fatal("Could not open kernel trace file: %s\n", strerror(errno));
-    }
+  int trace_marker_fd = get_trace_marker_fd();
+  if (trace_marker_fd == -1) {
+    return;
   }
 
   // If bionic tracing has been enabled, then write the message to the
@@ -87,12 +98,10 @@ ScopedTrace::ScopedTrace(const char* message) {
   int length = strlen(message);
   char buf[length + WRITE_OFFSET];
   size_t len = snprintf(buf, length + WRITE_OFFSET, "B|%d|%s", getpid(), message);
-  ssize_t wbytes = TEMP_FAILURE_RETRY(write(g_trace_marker_fd, buf, len));
 
-  // Error while writing
-  if (static_cast<size_t>(wbytes) != len) {
-    __libc_fatal("Could not write to kernel trace file: %s\n", strerror(errno));
-  }
+  // Tracing may stop just after checking property and before writing the message.
+  // So the write is acceptable to fail. See b/20666100.
+  TEMP_FAILURE_RETRY(write(trace_marker_fd, buf, len));
 }
 
 ScopedTrace::~ScopedTrace() {
@@ -100,10 +109,10 @@ ScopedTrace::~ScopedTrace() {
     return;
   }
 
-  ssize_t wbytes = TEMP_FAILURE_RETRY(write(g_trace_marker_fd, "E", 1));
-
-  // Error while writing
-  if (static_cast<size_t>(wbytes) != 1) {
-    __libc_fatal("Could not write to kernel trace file: %s\n", strerror(errno));
+  int trace_marker_fd = get_trace_marker_fd();
+  if (trace_marker_fd == -1) {
+    return;
   }
+
+  TEMP_FAILURE_RETRY(write(trace_marker_fd, "E", 1));
 }