Reviewed by Geoff Garen.
authorap@webkit.org <ap@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Jul 2008 05:55:57 +0000 (05:55 +0000)
committerap@webkit.org <ap@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Jul 2008 05:55:57 +0000 (05:55 +0000)
        Fix a leak of ThreadRegistrar objects.

        As the heap is usually deleted when registered threads still exist, ThreadSpecific doesn't
        have a chance to clean up per-thread object. Switched to native pthread calls, storing a
        plain pointer that doesn't require cleanup.

        * kjs/collector.cpp:
        (KJS::PlatformThread::PlatformThread):
        (KJS::Heap::Thread::Thread):
        (KJS::Heap::Heap):
        (KJS::Heap::~Heap):
        (KJS::Heap::registerThread):
        (KJS::Heap::unregisterThread):
        * kjs/collector.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@35440 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JavaScriptCore/ChangeLog
JavaScriptCore/kjs/collector.cpp
JavaScriptCore/kjs/collector.h

index 5841b7b..b39d615 100644 (file)
@@ -1,5 +1,24 @@
 2008-07-29  Alexey Proskuryakov  <ap@webkit.org>
 
+        Reviewed by Geoff Garen.
+
+        Fix a leak of ThreadRegistrar objects.
+
+        As the heap is usually deleted when registered threads still exist, ThreadSpecific doesn't
+        have a chance to clean up per-thread object. Switched to native pthread calls, storing a
+        plain pointer that doesn't require cleanup.
+
+        * kjs/collector.cpp:
+        (KJS::PlatformThread::PlatformThread):
+        (KJS::Heap::Thread::Thread):
+        (KJS::Heap::Heap):
+        (KJS::Heap::~Heap):
+        (KJS::Heap::registerThread):
+        (KJS::Heap::unregisterThread):
+        * kjs/collector.h:
+
+2008-07-29  Alexey Proskuryakov  <ap@webkit.org>
+
         Reviewed by Sam Weinig.
 
         https://bugs.webkit.org/show_bug.cgi?id=20169
index f2e39ec..8890a74 100644 (file)
 #include <wtf/HashCountedSet.h>
 #include <wtf/UnusedParam.h>
 
-#if USE(MULTIPLE_THREADS)
-#include <pthread.h>
-#include <wtf/Threading.h>
-#endif
-
 #if PLATFORM(DARWIN)
 
 #include <mach/mach_port.h>
@@ -69,8 +64,6 @@
 
 #if HAVE(PTHREAD_NP_H)
 #include <pthread_np.h>
-#else
-#include <pthread.h>
 #endif
 
 #endif
@@ -94,6 +87,35 @@ const size_t ALLOCATIONS_PER_COLLECTION = 4000;
 
 static void freeHeap(CollectorHeap*);
 
+#if USE(MULTIPLE_THREADS)
+
+#if PLATFORM(DARWIN)
+typedef mach_port_t PlatformThread;
+#elif PLATFORM(WIN_OS)
+struct PlatformThread {
+    PlatformThread(DWORD _id, HANDLE _handle) : id(_id), handle(_handle) {}
+    DWORD id;
+    HANDLE handle;
+};
+#endif
+
+class Heap::Thread {
+public:
+    Thread(pthread_t pthread, const PlatformThread& platThread, void* base) 
+        : posixThread(pthread)
+        , platformThread(platThread)
+        , stackBase(base)
+    {
+    }
+
+    Thread* next;
+    pthread_t posixThread;
+    PlatformThread platformThread;
+    void* stackBase;
+};
+
+#endif
+
 Heap::Heap(JSGlobalData* globalData)
     : m_markListSet(0)
 #if USE(MULTIPLE_THREADS)
@@ -101,6 +123,10 @@ Heap::Heap(JSGlobalData* globalData)
 #endif
     , m_globalData(globalData)
 {
+    int error = pthread_key_create(&m_currentThreadRegistrar, unregisterThread);
+    if (error)
+        CRASH();
+
     memset(&primaryHeap, 0, sizeof(CollectorHeap));
     memset(&numberHeap, 0, sizeof(CollectorHeap));
 }
@@ -117,6 +143,21 @@ Heap::~Heap()
 
     freeHeap(&primaryHeap);
     freeHeap(&numberHeap);
+
+#if USE(MULTIPLE_THREADS)
+#ifndef NDEBUG
+    int error =
+#endif
+        pthread_key_delete(m_currentThreadRegistrar);
+    ASSERT(!error);
+
+    MutexLocker registeredThreadsLock(m_registeredThreadsMutex);
+    for (Heap::Thread* t = m_registeredThreads; t;) {
+        Heap::Thread* next = t->next;
+        delete t;
+        t = next;
+    }
+#endif
 }
 
 static NEVER_INLINE CollectorBlock* allocateBlock()
@@ -407,16 +448,6 @@ static inline void* currentThreadStackBase()
 
 #if USE(MULTIPLE_THREADS)
 
-#if PLATFORM(DARWIN)
-typedef mach_port_t PlatformThread;
-#elif PLATFORM(WIN_OS)
-struct PlatformThread {
-    PlatformThread(DWORD _id, HANDLE _handle) : id(_id), handle(_handle) {}
-    DWORD id;
-    HANDLE handle;
-};
-#endif
-
 static inline PlatformThread getCurrentPlatformThread()
 {
 #if PLATFORM(DARWIN)
@@ -427,27 +458,12 @@ static inline PlatformThread getCurrentPlatformThread()
 #endif
 }
 
-class Heap::Thread {
-public:
-    Thread(pthread_t pthread, const PlatformThread& platThread, void* base) 
-        : posixThread(pthread)
-        , platformThread(platThread)
-        , stackBase(base)
-    {
-    }
-
-    Thread* next;
-    pthread_t posixThread;
-    PlatformThread platformThread;
-    void* stackBase;
-};
-
 void Heap::registerThread()
 {
-    if (m_currentThreadRegistrar)
+    if (pthread_getspecific(m_currentThreadRegistrar))
         return;
 
-    m_currentThreadRegistrar->set(new ThreadRegistrar(this));
+    pthread_setspecific(m_currentThreadRegistrar, this);
     Heap::Thread* thread = new Heap::Thread(pthread_self(), getCurrentPlatformThread(), currentThreadStackBase());
 
     MutexLocker lock(m_registeredThreadsMutex);
@@ -456,6 +472,12 @@ void Heap::registerThread()
     m_registeredThreads = thread;
 }
 
+void Heap::unregisterThread(void* p)
+{
+    if (p)
+        static_cast<Heap*>(p)->unregisterThread();
+}
+
 void Heap::unregisterThread()
 {
     pthread_t currentPosixThread = pthread_self();
index f59339d..13b6689 100644 (file)
@@ -29,7 +29,7 @@
 #include <wtf/OwnPtr.h>
 #include <wtf/Threading.h>
 #if USE(MULTIPLE_THREADS)
-#include <wtf/ThreadSpecific.h>
+#include <pthread.h>
 #endif
 
 namespace KJS {
@@ -131,20 +131,12 @@ namespace KJS {
         HashSet<ArgList*>* m_markListSet;
 
 #if USE(MULTIPLE_THREADS)
+        static void unregisterThread(void*);
         void unregisterThread();
 
-        class ThreadRegistrar : Noncopyable {
-        public:
-            ThreadRegistrar(Heap* heap) : m_heap(heap) {}
-            ~ThreadRegistrar() { m_heap->unregisterThread(); }
-
-        private:
-            Heap* m_heap;
-        };
-
         Mutex m_registeredThreadsMutex;
         Thread* m_registeredThreads;
-        WTF::ThreadSpecific<OwnPtr<ThreadRegistrar> > m_currentThreadRegistrar;
+        pthread_key_t m_currentThreadRegistrar;
 #endif
 
         JSGlobalData* m_globalData;