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>

1 files changed, 15 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*>(