From 2058faaa3bd7604e1b8c25b50efd23893ec845d4 Mon Sep 17 00:00:00 2001 From: Guillaume MM Date: Sat, 13 May 2017 01:00:30 +0200 Subject: [PATCH] Prevent false positives in external modifications When the Buffer is notified to be externally modified, check that the file contents have changed using the checksum. Document the shortcoming of FileMonitorBlocker. Fixes #10642. --- src/Buffer.cpp | 12 ++++++++++-- src/support/FileMonitor.h | 15 +++++++++++---- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/Buffer.cpp b/src/Buffer.cpp index 9f6b20f7b7..9eded216f1 100644 --- a/src/Buffer.cpp +++ b/src/Buffer.cpp @@ -386,7 +386,7 @@ public: void fileExternallyModified(bool modified) const; /// Block notifications of external modifications - FileMonitorBlocker blockFileMonitor() { return file_monitor_->block(10); } + FileMonitorBlocker blockFileMonitor() { return file_monitor_->block(); } private: /// So we can force access via the accessors. @@ -5341,8 +5341,16 @@ void Buffer::Impl::refreshFileMonitor() void Buffer::Impl::fileExternallyModified(bool modified) const { - if (modified) + if (modified) { + // prevent false positives, because FileMonitorBlocker is not enough on + // OSX. + if (filename.exists() && checksum_ == filename.checksum()) { + LYXERR(Debug::FILES, "External modification but " + "checksum unchanged: " << filename); + return; + } lyx_clean = bak_clean = false; + } externally_modified_ = modified; if (wa_) wa_->updateTitles(); diff --git a/src/support/FileMonitor.h b/src/support/FileMonitor.h index 49f12cba72..5e516bb012 100644 --- a/src/support/FileMonitor.h +++ b/src/support/FileMonitor.h @@ -167,11 +167,18 @@ public: /// absolute path being tracked std::string const & filename() { return monitor_->filename(); } /// Creates a guard that blocks notifications. Copyable. Notifications from - /// this monitor are blocked as long as there are copies around. + /// this monitor are blocked as long as there are copies of the + /// FileMonitorBlocker around. /// \param delay is the amount waited in ms after expiration of the guard - /// before reconnecting. This delay thing is to deal with asynchronous - /// notifications in a not so elegant fashion. But it can also be used to - /// slow down incoming events. + /// before reconnecting. It can be used to slow down incoming events + /// accordingly. A value of 0 is still made asynchronous, because of the + /// fundamentally asynchronous nature of QFileSystemWatcher. To catch one's + /// own file operations, a value of 0 for delay is sufficient with the + /// inotify backend (e.g. Linux); for OSX (kqueue), a value of 100ms is + /// unsufficient and more tests need to be done in combination with + /// flushing/syncing to disk in order to understand how to catch one's own + /// operations reliably. No feedback from Windows yet. See + /// . FileMonitorBlocker block(int delay = 0); /// Make sure the good file is being monitored, after e.g. a move or a /// deletion. See . This is