Merge branch 'fork-safe' (issue #110)

4 files changed, 68 insertions, 6 deletions

diff --git a/common/os_posix.cpp b/common/os_posix.cpp
index bd15bbac..5ec8b363 100644
--- a/common/os_posix.cpp
+++ b/common/os_posix.cpp
@@ -173,8 +173,16 @@ log(const char *format, ...)
 #ifdef ANDROID
     __android_log_vprint(ANDROID_LOG_DEBUG, "apitrace", format, ap);
 #else
-    vfprintf(stderr, format, ap);
-    fflush(stderr);
+    static FILE *log = NULL;
+    if (!log) {
+        // Duplicate stderr file descriptor, to prevent applications from
+        // redirecting our debug messages to somewhere else.
+        //
+        // Another alternative would be to log to /dev/tty when available.
+        log = fdopen(dup(STDERR_FILENO), "at");
+    }
+    vfprintf(log, format, ap);
+    fflush(log);
 #endif
     va_end(ap);
     logging = false;
diff --git a/common/os_process.hpp b/common/os_process.hpp
index ba53408c..e1b1df17 100644
--- a/common/os_process.hpp
+++ b/common/os_process.hpp
@@ -33,6 +33,9 @@
 
 #ifdef _WIN32
 #include <windows.h>
+#else
+#include <sys/types.h>
+#include <unistd.h>
 #endif
 
 #include "os.hpp"
@@ -41,7 +44,26 @@
 namespace os {
 
 
-inline void
+typedef
+#ifdef _WIN32
+   DWORD
+#else
+   pid_t
+#endif
+ProcessId;
+
+
+static inline ProcessId
+getCurrentProcessId(void) {
+#ifdef _WIN32
+    return GetCurrentProcessId();
+#else
+    return getpid();
+#endif
+}
+
+
+static inline void
 setEnvironment(const char *name, const char *value) {
 #ifdef _WIN32
     SetEnvironmentVariableA(name, value);
@@ -51,7 +73,7 @@ setEnvironment(const char *name, const char *value) {
 }
 
 
-inline void
+static inline void
 unsetEnvironment(const char *name) {
 #ifdef _WIN32
     SetEnvironmentVariableA(name, NULL);
diff --git a/common/trace_writer_local.cpp b/common/trace_writer_local.cpp
index 757e9c0f..d2ff3b5f 100644
--- a/common/trace_writer_local.cpp
+++ b/common/trace_writer_local.cpp
@@ -63,6 +63,8 @@ static void exceptionCallback(void)
 LocalWriter::LocalWriter() :
     acquired(0)
 {
+    os::log("apitrace: loaded\n");
+
     // Install the signal handlers as early as possible, to prevent
     // interfering with the application's signal handling.
     os::setExceptionCallback(exceptionCallback);
@@ -71,6 +73,7 @@ LocalWriter::LocalWriter() :
 LocalWriter::~LocalWriter()
 {
     os::resetExceptionCallback();
+    checkProcessId();
 }
 
 void
@@ -122,6 +125,8 @@ LocalWriter::open(void) {
         os::abort();
     }
 
+    pid = os::getCurrentProcessId();
+
 #if 0
     // For debugging the exception handler
     *((int *)0) = 0;
@@ -133,10 +138,25 @@ static uintptr_t next_thread_num = 1;
 static OS_THREAD_SPECIFIC_PTR(void)
 thread_num;
 
+void LocalWriter::checkProcessId(void) {
+    if (m_file->isOpened() &&
+        os::getCurrentProcessId() != pid) {
+        // We are a forked child process that inherited the trace file, so
+        // create a new file.  We can't call any method of the current
+        // file, as it may cause it to flush and corrupt the parent's
+        // trace, so we effectively leak the old file object.
+        m_file = File::createSnappy();
+        // Don't want to open the same file again
+        os::unsetEnvironment("TRACE_FILE");
+        open();
+    }
+}
+
 unsigned LocalWriter::beginEnter(const FunctionSig *sig) {
     mutex.lock();
     ++acquired;
 
+    checkProcessId();
     if (!m_file->isOpened()) {
         open();
     }
@@ -185,8 +205,12 @@ void LocalWriter::flush(void) {
     } else {
         ++acquired;
         if (m_file->isOpened()) {
-            os::log("apitrace: flushing trace due to an exception\n");
-            m_file->flush();
+            if (os::getCurrentProcessId() != pid) {
+                os::log("apitrace: ignoring exception in child process\n");
+            } else {
+                os::log("apitrace: flushing trace due to an exception\n");
+                m_file->flush();
+            }
         }
         --acquired;
     }
diff --git a/common/trace_writer_local.hpp b/common/trace_writer_local.hpp
index cc5dda01..815e3c0f 100644
--- a/common/trace_writer_local.hpp
+++ b/common/trace_writer_local.hpp
@@ -34,6 +34,7 @@
 #include <stdint.h>
 
 #include "os_thread.hpp"
+#include "os_process.hpp"
 #include "trace_writer.hpp"
 
 
@@ -70,6 +71,13 @@ namespace trace {
         os::recursive_mutex mutex;
         int acquired;
 
+        /**
+         * ID of the processed that opened the trace file.
+         */
+        os::ProcessId pid;
+
+        void checkProcessId();
+
     public:
         /**
          * Should never called directly -- use localWriter singleton below