Changeset 184072 in webkit

Timestamp:

May 11, 2015, 3:21:40 AM (11 years ago)

Author:

Carlos Garcia Campos

Message:

[GTK] WorkQueue objects are not released
​https://bugs.webkit.org/show_bug.cgi?id=144824

Reviewed by Žan Doberšek.

Do not keep a reference of the WorkQueue for the entire life of
its worker thread, since every task scheduled on the WorkQueue
already takes a reference. Instead, take a reference of the main
loop to make sure that when the worker thread starts, the main
loop hasn't been released to avoid runtime warnings (see
webkit.org/b/140998). Also removed the g_main_context_pop_thread_default()
from the thread body, since the thread-specific context queue will
be freed anyway when the thread exits.
If the WorkQueue is released early, before the thread has started,
schedule a main loop quit in the context, to make sure it will
be the first thing run by the main loop and the thread will exit.

• wtf/WorkQueue.h: Remove unused event loop mutex.
• wtf/gtk/WorkQueueGtk.cpp:

(WTF::WorkQueue::platformInitialize):
(WTF::WorkQueue::platformInvalidate):

Location:

trunk/Source/WTF

Files:

• ChangeLog (modified) (1 diff)
• wtf/WorkQueue.h (modified) (1 diff)
• wtf/gtk/WorkQueueGtk.cpp (modified) (2 diffs)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2015-05-11  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] WorkQueue objects are not released
+        https://bugs.webkit.org/show_bug.cgi?id=144824
+
+        Reviewed by Žan Doberšek.
+
+        Do not keep a reference of the WorkQueue for the entire life of
+        its worker thread, since every task scheduled on the WorkQueue
+        already takes a reference. Instead, take a reference of the main
+        loop to make sure that when the worker thread starts, the main
+        loop hasn't been released to avoid runtime warnings (see
+        webkit.org/b/140998). Also removed the g_main_context_pop_thread_default()
+        from the thread body, since the thread-specific context queue will
+        be freed anyway when the thread exits.
+        If the WorkQueue is released early, before the thread has started,
+        schedule a main loop quit in the context, to make sure it will
+        be the first thing run by the main loop and the thread will exit.
+
+        * wtf/WorkQueue.h: Remove unused event loop mutex.
+        * wtf/gtk/WorkQueueGtk.cpp:
+        (WTF::WorkQueue::platformInitialize):
+        (WTF::WorkQueue::platformInvalidate):
+
 2015-05-09  Yoav Weiss  <yoav@yoav.ws>
 

trunk/Source/WTF/wtf/WorkQueue.h

@@ -107 +107 @@
     ThreadIdentifier m_workQueueThread;
     GRefPtr<GMainContext> m_eventContext;
-    Mutex m_eventLoopLock;
     GRefPtr<GMainLoop> m_eventLoop;
     GMainLoopSource m_socketEventSource;

trunk/Source/WTF/wtf/gtk/WorkQueueGtk.cpp

@@ -56 +56 @@
         threadName += strlen(threadName) - kVisualStudioThreadNameLimit;
 
-    RefPtr<WorkQueue> protector(this);
-    m_workQueueThread = createThread(threadName, [protector] {
-        g_main_context_push_thread_default(protector->m_eventContext.get());
-        g_main_loop_run(protector->m_eventLoop.get());
-        g_main_context_pop_thread_default(protector->m_eventContext.get());
+    GRefPtr<GMainLoop> eventLoop(m_eventLoop.get());
+    m_workQueueThread = createThread(threadName, [eventLoop] {
+        g_main_context_push_thread_default(g_main_loop_get_context(eventLoop.get()));
+        g_main_loop_run(eventLoop.get());
     });
 }
@@ -71 +70 @@
     }
 
-    MutexLocker locker(m_eventLoopLock);
     if (m_eventLoop) {
         if (g_main_loop_is_running(m_eventLoop.get()))
             g_main_loop_quit(m_eventLoop.get());
-        m_eventLoop.clear();
+        else {
+            // The thread hasn't started yet, so schedule a main loop quit to ensure the thread finishes.
+            GMainLoop* eventLoop = m_eventLoop.get();
+            GMainLoopSource::scheduleAndDeleteOnDestroy("[WebKit] WorkQueue quit main loop", [eventLoop] { g_main_loop_quit(eventLoop); },
+                G_PRIORITY_HIGH, nullptr, m_eventContext.get());
+        }
+        m_eventLoop = nullptr;
     }
 
-    m_eventContext.clear();
+    m_eventContext = nullptr;
 }