Changeset 66741 in webkit for trunk/JavaScriptCore/wtf/FastMalloc.cpp

Timestamp:

Sep 3, 2010, 11:02:51 AM (15 years ago)

Author:

[email protected]

Message:

2010-09-03 Alexey Proskuryakov <​[email protected]>

Reviewed by Darin Adler.

​https://bugs.webkit.org/show_bug.cgi?id=45135
<rdar://problem/7823714> TCMalloc_PageHeap doesn't hold a mutex while manipulating shared data

• wtf/FastMalloc.cpp: (WTF::TCMalloc_PageHeap::initializeScavenger): Make sure to create a non-recursive mutex regardless of platform default, so that we can assert that it's held (this is for platforms that don't have libdispatch). (WTF::TCMalloc_PageHeap::signalScavenger): Assert that the mutex is held, so we can look at m_scavengeThreadActive. For platforms that have libdispatch, assert that pageheap_lock is held. (WTF::TCMalloc_PageHeap::periodicScavenge): Make sure that pageheap_lock is held before manipulating m_scavengeThreadActive. Otherwise, there is an obvious race condition, and we can make unbalanced calls to dispatch_resume().

File:

• trunk/JavaScriptCore/wtf/FastMalloc.cpp (modified) (4 diffs)

trunk/JavaScriptCore/wtf/FastMalloc.cpp

@@ -1493 +1493 @@
 void TCMalloc_PageHeap::initializeScavenger()
 {
-  pthread_mutex_init(&m_scavengeMutex, 0);
-  pthread_cond_init(&m_scavengeCondition, 0);
-  m_scavengeThreadActive = true;
-  pthread_t thread;
-  pthread_create(&thread, 0, runScavengerThread, this);
+    // Create a non-recursive mutex.
+#if PTHREAD_MUTEX_NORMAL == PTHREAD_MUTEX_DEFAULT
+    pthread_mutex_init(&m_scavengeMutex, 0);
+#else
+    pthread_mutexattr_t attr;
+    pthread_mutexattr_init(&attr);
+    pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_NORMAL);
+
+    pthread_mutex_init(&m_scavengeMutex, &attr);
+
+    pthread_mutexattr_destroy(&attr);
+#endif
+
+    pthread_cond_init(&m_scavengeCondition, 0);
+    m_scavengeThreadActive = true;
+    pthread_t thread;
+    pthread_create(&thread, 0, runScavengerThread, this);
 }
 
@@ -1511 +1523 @@
 ALWAYS_INLINE void TCMalloc_PageHeap::signalScavenger()
 {
-  if (!m_scavengeThreadActive && shouldScavenge())
-    pthread_cond_signal(&m_scavengeCondition);
+    // m_scavengeMutex should be held before accessing m_scavengeThreadActive.
+    ASSERT(pthread_mutex_trylock(m_scavengeMutex));
+    if (!m_scavengeThreadActive && shouldScavenge())
+        pthread_cond_signal(&m_scavengeCondition);
 }
 
@@ -1529 +1543 @@
 ALWAYS_INLINE void TCMalloc_PageHeap::signalScavenger()
 {
-  if (!m_scavengingScheduled && shouldScavenge()) {
-    m_scavengingScheduled = true;
-    dispatch_resume(m_scavengeTimer);
-  }
+    ASSERT(IsHeld(pageheap_lock));
+    if (!m_scavengingScheduled && shouldScavenge()) {
+        m_scavengingScheduled = true;
+        dispatch_resume(m_scavengeTimer);
+    }
 }
 
@@ -2398 +2413 @@
 void TCMalloc_PageHeap::periodicScavenge()
 {
-  {
     SpinLockHolder h(&pageheap_lock);
     pageheap->scavenge();
-  }
-
-  if (!shouldScavenge()) {
-    m_scavengingScheduled = false;
-    dispatch_suspend(m_scavengeTimer);
-  }
+
+    if (!shouldScavenge()) {
+        m_scavengingScheduled = false;
+        dispatch_suspend(m_scavengeTimer);
+    }
 }
 #endif // HAVE(DISPATCH_H)