[GLIB] Test FileMonitorTest.DetectChangeAndThenDelete sometimes crashes
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 21 Nov 2017 16:53:41 +0000 (16:53 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 21 Nov 2017 16:53:41 +0000 (16:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179909

Reviewed by Michael Catanzaro.

The problem sems to be that the GFileMonitor is created in the main thread, but destroyed in the WorkQueue
thread. We can create the monitor in the WorkQueue thread and do the monitoring there.

Fixes unit test FileMonitorTest.DetectChangeAndThenDelete.

* platform/glib/FileMonitorGLib.cpp:
(WebCore::FileMonitor::FileMonitor): Create the GFileMonitor in the WorkQueue.
(WebCore::FileMonitor::didChange): No need to dispatch the handler in the WorkQueue, since this is now called in
the WorkQueue.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/glib/FileMonitorGLib.cpp

index 6c1ced4..db8807d 100644 (file)
@@ -1,3 +1,20 @@
+2017-11-21  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GLIB] Test FileMonitorTest.DetectChangeAndThenDelete sometimes crashes
+        https://bugs.webkit.org/show_bug.cgi?id=179909
+
+        Reviewed by Michael Catanzaro.
+
+        The problem sems to be that the GFileMonitor is created in the main thread, but destroyed in the WorkQueue
+        thread. We can create the monitor in the WorkQueue thread and do the monitoring there.
+
+        Fixes unit test FileMonitorTest.DetectChangeAndThenDelete.
+
+        * platform/glib/FileMonitorGLib.cpp:
+        (WebCore::FileMonitor::FileMonitor): Create the GFileMonitor in the WorkQueue.
+        (WebCore::FileMonitor::didChange): No need to dispatch the handler in the WorkQueue, since this is now called in
+        the WorkQueue.
+
 2017-11-21  Zan Dobersek  <zdobersek@igalia.com>
 
         [Cairo] drawNativeImage(), drawPattern() in CairoOperations should operate directly with cairo_surface_t objects
index 94924e6..84d496a 100644 (file)
@@ -33,21 +33,24 @@ namespace WebCore {
 
 FileMonitor::FileMonitor(const String& path, Ref<WorkQueue>&& handlerQueue, WTF::Function<void(FileChangeType)>&& modificationHandler)
     : m_handlerQueue(WTFMove(handlerQueue))
+    , m_modificationHandler(WTFMove(modificationHandler))
 {
-    if (path.isEmpty() || !modificationHandler)
+    if (path.isEmpty() || !m_modificationHandler)
         return;
 
-    auto file = adoptGRef(g_file_new_for_path(FileSystem::fileSystemRepresentation(path).data()));
     m_cancellable = adoptGRef(g_cancellable_new());
-    GUniqueOutPtr<GError> error;
-    m_platformMonitor = adoptGRef(g_file_monitor(file.get(), G_FILE_MONITOR_NONE, m_cancellable.get(), &error.outPtr()));
-    if (!m_platformMonitor) {
-        WTFLogAlways("Failed to create a monitor for path %s: %s", path.utf8().data(), error->message);
-        return;
-    }
-
-    m_modificationHandler = WTFMove(modificationHandler);
-    g_signal_connect(m_platformMonitor.get(), "changed", G_CALLBACK(fileChangedCallback), this);
+    m_handlerQueue->dispatch([this, cancellable = m_cancellable, path = path.isolatedCopy()] {
+        if (g_cancellable_is_cancelled(cancellable.get()))
+            return;
+        auto file = adoptGRef(g_file_new_for_path(FileSystem::fileSystemRepresentation(path).data()));
+        GUniqueOutPtr<GError> error;
+        m_platformMonitor = adoptGRef(g_file_monitor(file.get(), G_FILE_MONITOR_NONE, m_cancellable.get(), &error.outPtr()));
+        if (!m_platformMonitor) {
+            WTFLogAlways("Failed to create a monitor for path %s: %s", path.utf8().data(), error->message);
+            return;
+        }
+        g_signal_connect(m_platformMonitor.get(), "changed", G_CALLBACK(fileChangedCallback), this);
+    });
 }
 
 FileMonitor::~FileMonitor()
@@ -72,15 +75,15 @@ void FileMonitor::fileChangedCallback(GFileMonitor*, GFile*, GFile*, GFileMonito
 
 void FileMonitor::didChange(FileChangeType type)
 {
-    m_handlerQueue->dispatch([this, cancellable = m_cancellable, type] {
-        if (g_cancellable_is_cancelled(cancellable.get()))
-            return;
-        m_modificationHandler(type);
-        if (type == FileChangeType::Removal) {
-            g_cancellable_cancel(m_cancellable.get());
-            m_platformMonitor = nullptr;
-        }
-    });
+    ASSERT(!isMainThread());
+    if (g_cancellable_is_cancelled(m_cancellable.get())) {
+        m_platformMonitor = nullptr;
+        return;
+    }
+
+    m_modificationHandler(type);
+    if (type == FileChangeType::Removal)
+        m_platformMonitor = nullptr;
 }
 
 } // namespace WebCore