[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