Race condition in trace leads to double-delete.

Change-Id: I4fc4acc8f84de151d1d9600802539c18c6aa7851
Reviewed-on: http://gfw160.aus.stglabs.ibm.com:8080/gerrit/7277
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>
Reviewed-by: Thi N. Tran <thi@us.ibm.com>
Tested-by: Jenkins Server
Reviewed-by: A. Patrick Williams III <iawillia@us.ibm.com>

2 files changed, 19 insertions, 22 deletions

diff --git a/src/usr/trace/buffer.C b/src/usr/trace/buffer.C
index 1b1d64736..a405f54c7 100644
--- a/src/usr/trace/buffer.C
+++ b/src/usr/trace/buffer.C
@@ -254,28 +254,6 @@ namespace TRACE
             BufferPage* newPage = BufferPage::allocate();
             newPage->prev = first;
 
-            // If there is a page already, update its next pointer to point
-            // back at this new page.
-            if (first)
-            {
-                if (!__sync_bool_compare_and_swap(&first->next,
-                                                  NULL,
-                                                  newPage))
-                {
-                    // Someone beat us to allocating the page, release it.
-                    BufferPage::deallocate(newPage);
-                    do
-                    {
-                        pagesAllocated = iv_pagesAlloc;
-                        newPagesAllocated = pagesAllocated - 1;
-                    }
-                    while (!__sync_bool_compare_and_swap(&iv_pagesAlloc,
-                                                         pagesAllocated,
-                                                         newPagesAllocated));
-                    continue;
-                }
-            }
-
             // Now we have a page allocated, claim our entry first and then
             // hook it up to master list.
             l_entry = newPage->claimEntry(i_size);
@@ -302,6 +280,21 @@ namespace TRACE
                 continue;
             }
 
+            // If there was a page already, update its next pointer to point
+            // back at this new page.
+            if (first)
+            {
+                if (!__sync_bool_compare_and_swap(&first->next,
+                                                  NULL,
+                                                  newPage))
+                {
+                    // We were the first one to update iv_firstPage, so
+                    // first->next should have been NULL and nobody was
+                    // suppose to touch it.
+                    assert(false);
+                }
+            }
+
             // Since we allocated a page, signal any other tasks that might be
             // blocked waiting for a page to show up.
             futex_wake(reinterpret_cast<uint64_t*>(
diff --git a/src/usr/trace/daemon/daemon.C b/src/usr/trace/daemon/daemon.C
index 6351381c0..51071ca39 100644
--- a/src/usr/trace/daemon/daemon.C
+++ b/src/usr/trace/daemon/daemon.C
@@ -35,6 +35,7 @@
 #include <initservice/initserviceif.H>
 
 #include <sys/msg.h>
+#include <sys/task.h>
 #include <kernel/console.H>
 #include <util/align.H>
 
@@ -76,6 +77,9 @@ namespace TRACEDAEMON
 
     void Daemon::execute()
     {
+        // Mark as an independent daemon so if it crashes we terminate.
+        task_detach();
+
         // Mark the daemon as started in the interface.
         iv_service = Service::getGlobalInstance();
         iv_service->iv_daemon->start();