[LyX/master] Fix race condition between export and preview machinery

Pavel Sanda <[email protected]> Mon, 20 Jul 2026 21:40:04 +0000
Newsgroups gmane.editors.lyx.cvs
Message-ID <[email protected]>
commit dbc8bf88b9de7e535615887bc6fecab9a9370efb
Author: Pavel Sanda <[email protected]>
Date:   Mon Jul 20 12:00:00 2026 +0200

    Fix race condition between export and preview machinery
    
    Launch export while images are being loaded -> race between main and export threads:
     malloc(): unaligned tcache chunk detected, kaboom.
    
    This race was always there, but surfaced with higher probability once we
    introduced more aggressive image preload (e0dd8db6ade84).
    
    Both export (independent thread) and image preview machinery (runs on main
    app thread) converge on Formats::getFormatFromFile() which itself uses
    non-thread-safe singleton: the shared libmagic cookie.
    
    Light-touch approach: keep the main-thread preview machinery off the shared
    converter/format singletons while a threaded export owns them: ongoing
    previews are not written on disk (will regenerate again) and new previews
    are not queued.
    
    Heavier approach (not implemented) could track ongoing previews to finish
    cache after exports, but the code gets unnecessarily complicated.
    
    Mutexes would be cheapest code-wise, but need to be both on
    ConverterCache's pimpl_->cache and getFormatFromFile()'s magic
    and it feels non robust in long-term.
    
    Assisted-by: Claude Opus 4.8
---
 src/frontends/qt/GuiView.cpp       |  5 +++++
 src/graphics/GraphicsCacheItem.cpp | 18 ++++++++++++++++--
 src/graphics/GraphicsCacheItem.h   |  5 +++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/frontends/qt/GuiView.cpp b/src/frontends/qt/GuiView.cpp
index aab3394c60..c199faebc8 100644
--- a/src/frontends/qt/GuiView.cpp
+++ b/src/frontends/qt/GuiView.cpp
@@ -69,6 +69,7 @@
 #include "Toolbars.h"
 #include "version.h"
 
+#include "graphics/GraphicsCacheItem.h"
 #include "graphics/PreviewLoader.h"
 
 #include "support/convert.h"
@@ -414,6 +415,8 @@ public:
 			// interface.
 			return;
 		}
+		// not thread-safe for converter/format singletons
+		graphics::CacheItem::setBackgroundExportRunning(true);
 		processing_thread_watcher_.setFuture(f);
 	}
 
@@ -949,6 +952,8 @@ void GuiView::processingThreadStarted()
 
 void GuiView::processingThreadFinished()
 {
+	graphics::CacheItem::setBackgroundExportRunning(false);
+
 	QFutureWatcher<Buffer::ExportStatus> const * watcher =
 		static_cast<QFutureWatcher<Buffer::ExportStatus> const *>(sender());
 
diff --git a/src/graphics/GraphicsCacheItem.cpp b/src/graphics/GraphicsCacheItem.cpp
index 1c15babafc..fe941a2f5d 100644
--- a/src/graphics/GraphicsCacheItem.cpp
+++ b/src/graphics/GraphicsCacheItem.cpp
@@ -121,6 +121,8 @@ public:
 
 	///
 	unique_ptr<Converter> converter_;
+
+	inline static bool background_export_running_ = false;
 };
 
 
@@ -135,6 +137,12 @@ CacheItem::~CacheItem()
 }
 
 
+void CacheItem::setBackgroundExportRunning(bool running)
+{
+	Impl::background_export_running_ = running;
+}
+
+
 FileName const & CacheItem::filename() const
 {
 	return pimpl_->filename_;
@@ -285,8 +293,11 @@ void CacheItem::Impl::imageConverted(bool success)
 		return;
 	}
 
-	// Add the converted file to the file cache
-	ConverterCache::get().add(filename_, to_, file_to_load_);
+	// Add the converted file to the file cache. Skips while a threaded
+	// export owns the converter/format singletons on another thread -
+	// the image still loads, just not written to the on-disk cache.
+	if (!background_export_running_)
+		ConverterCache::get().add(filename_, to_, file_to_load_);
 
 	setStatus(loadImage() ? Loaded : ErrorLoading);
 }
@@ -353,6 +364,9 @@ static string const findTargetFormat(FormatList const & format_list, string cons
 
 bool CacheItem::Impl::tryDisplayFormat(FileName & filename, string & from)
 {
+	if (background_export_running_)
+		return false;
+
 	// First, check that the file exists!
 	filename_.refresh();
 	if (!filename_.isReadableFile()) {
diff --git a/src/graphics/GraphicsCacheItem.h b/src/graphics/GraphicsCacheItem.h
index a018b3931d..02f97d3b0b 100644
--- a/src/graphics/GraphicsCacheItem.h
+++ b/src/graphics/GraphicsCacheItem.h
@@ -68,6 +68,11 @@ public:
 	/// perform a modification check asynchronously
 	void checkModifiedAsync() const;
 
+	/// Export uses the non-thread-safe converter/format singletons (libmagic,
+	/// ConverterCache) on a worker thread. Routine signals "export started" to
+	/// image preview machinery (which lives on THIS main app thread, so no race).
+	static void setBackgroundExportRunning(bool running);
+
 	/** Get the image associated with filename().
 	 *  If the image is not yet loaded, returns 0.
 	 *  This routine returns a pointer to const; if you want to modify it,
-- 
lyx-cvs mailing list
[email protected]
https://lists.lyx.org/mailman/listinfo/lyx-cvs