From: Guillaume MM <gm@lyx.org>
Date: Tue, 13 Nov 2018 22:10:35 +0000 (+0100)
Subject: Fix segfault after deleting monitor
X-Git-Tag: 2.3.3~136
X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=a8ad4002a06a42d395de2595c5c3254256bccd10;p=features.git

Fix segfault after deleting monitor

The boost signal was sent synchronously, and so made the Qt signal to be posted
in FileMonitor::changed after the boost signal returned, so after the sender was
possibly destroyed.

The solution is to make the boost signal asynchronous using the Qt event loop.

Thanks to Scott Kostyshak for the report and MWE.

(cherry picked from commit 131f4b92bac3ecb75b47c266dfa3d8543bd4d578)
---

diff --git a/src/insets/RenderPreview.cpp b/src/insets/RenderPreview.cpp
index df0ac64d7e..a2162507f0 100644
--- a/src/insets/RenderPreview.cpp
+++ b/src/insets/RenderPreview.cpp
@@ -304,6 +304,7 @@ void RenderMonitoredPreview::startMonitoring() const
 {
 	if (!monitoring()) {
 		monitor_ = FileSystemWatcher::activeMonitor(filename_);
+		// Disconnected at the same time as this is destroyed.
 		monitor_->connect([this](bool /* exists */){ changed_(); });
 	}
 }
diff --git a/src/support/FileMonitor.cpp b/src/support/FileMonitor.cpp
index c703d2bba7..a3b17ad012 100644
--- a/src/support/FileMonitor.cpp
+++ b/src/support/FileMonitor.cpp
@@ -176,7 +176,14 @@ FileMonitor::FileMonitor(std::shared_ptr<FileMonitorGuard> monitor)
 
 void FileMonitor::connectToFileMonitorGuard()
 {
+	// Connections need to be asynchronous because the receiver can own this
+	// object and therefore is allowed to delete it.
+	// Qt signal:
 	QObject::connect(monitor_.get(), SIGNAL(fileChanged(bool)),
+	                 this, SIGNAL(fileChanged(bool)),
+	                 Qt::QueuedConnection);
+	// Boost signal:
+	QObject::connect(this, SIGNAL(fileChanged(bool)),
 	                 this, SLOT(changed(bool)));
 }
 
@@ -187,18 +194,10 @@ signals2::connection FileMonitor::connect(slot const & slot)
 }
 
 
-void FileMonitor::disconnect()
-{
-	fileChanged_.disconnect_all_slots();
-	QObject::disconnect(this, SIGNAL(fileChanged(bool)));
-}
-
-
 void FileMonitor::changed(bool const exists)
 {
 	// emit boost signal
 	fileChanged_(exists);
-	Q_EMIT fileChanged(exists);
 }
 
 
@@ -244,7 +243,7 @@ void ActiveFileMonitor::checkModified()
 		}
 	}
 	if (changed)
-		FileMonitor::changed(exists);
+		Q_EMIT FileMonitor::fileChanged(exists);
 	QTimer::singleShot(interval_, this, SLOT(clearCooldown()));
 }
 
diff --git a/src/support/FileMonitor.h b/src/support/FileMonitor.h
index 14aa834c80..07a8dc532c 100644
--- a/src/support/FileMonitor.h
+++ b/src/support/FileMonitor.h
@@ -60,10 +60,6 @@ typedef std::unique_ptr<ActiveFileMonitor> ActiveFileMonitorPtr;
 ///   monitor.connect(...);
 /// (stops watching the first)
 ///
-/// Reset connections:
-///   monitor.disconnect();
-///   or the disconnect method of the connection object for the boost signal.
-///
 class FileSystemWatcher
 {
 public:
@@ -137,9 +133,6 @@ public:
 	typedef sig::slot_type slot;
 	/// Connect and you'll be informed when the file has changed.
 	signals2::connection connect(slot const &);
-	/// disconnect all slots connected to the boost signal fileChanged_ or to
-	/// the qt signal fileChanged()
-	void disconnect();
 	/// absolute path being tracked
 	std::string const & filename() { return monitor_->filename(); }
 	/// Make sure the good file is being monitored, after e.g. a move or a