From fc9f83966cd050c9de92356df6a6508c37ecfab9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 03:23:54 +0000 Subject: [PATCH] HighlighterEngine: remove background warm-up thread The worker read the Buffer (PieceTable pieces and add buffer) while the main thread edited it with no shared lock, and kept a raw Buffer* that dangled once the buffer vector reallocated or shifted. PrefetchViewport now only highlights the visible rows synchronously. Audit finding M2. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB --- syntax/HighlighterEngine.cc | 78 +------------------------------------ syntax/HighlighterEngine.h | 35 +++-------------- 2 files changed, 8 insertions(+), 105 deletions(-) diff --git a/syntax/HighlighterEngine.cc b/syntax/HighlighterEngine.cc index b890b79..6cfab06 100644 --- a/syntax/HighlighterEngine.cc +++ b/syntax/HighlighterEngine.cc @@ -1,26 +1,12 @@ #include "HighlighterEngine.h" #include "../Buffer.h" #include "LanguageHighlighter.h" -#include namespace kte { HighlighterEngine::HighlighterEngine() = default; -HighlighterEngine::~HighlighterEngine() -{ - // stop background worker - if (worker_running_.load()) { - { - std::lock_guard lock(mtx_); - worker_running_.store(false); - has_request_ = true; // wake it up to exit - } - cv_.notify_one(); - if (worker_.joinable()) - worker_.join(); - } -} +HighlighterEngine::~HighlighterEngine() = default; void @@ -168,53 +154,9 @@ HighlighterEngine::InvalidateFrom(int row) } -void -HighlighterEngine::ensure_worker_started() const -{ - if (worker_running_.load()) - return; - worker_running_.store(true); - worker_ = std::thread([this]() { - this->worker_loop(); - }); -} - - -void -HighlighterEngine::worker_loop() const -{ - std::unique_lock lock(mtx_); - while (worker_running_.load()) { - cv_.wait(lock, [this]() { - return has_request_ || !worker_running_.load(); - }); - if (!worker_running_.load()) - break; - WarmRequest req = pending_; - has_request_ = false; - // Copy locals then release lock while computing - lock.unlock(); - if (req.buf) { - int start = std::max(0, req.start_row); - int end = std::max(start, req.end_row); - int skip_f = std::min(req.skip_first, req.skip_last); - int skip_l = std::max(req.skip_first, req.skip_last); - for (int r = start; r <= end; ++r) { - // Avoid touching rows that the foreground just computed/drew. - if (r >= skip_f && r <= skip_l) - continue; - // Compute line; GetLine is thread-safe and will refresh caches. - (void) this->GetLine(*req.buf, r, req.version); - } - } - lock.lock(); - } -} - - void HighlighterEngine::PrefetchViewport(const Buffer &buf, int first_row, int row_count, std::uint64_t buf_version, - int warm_margin) const + int /*warm_margin*/) const { if (row_count <= 0) return; @@ -230,21 +172,5 @@ HighlighterEngine::PrefetchViewport(const Buffer &buf, int first_row, int row_co for (int r = start; r <= end; ++r) { (void) GetLine(buf, r, buf_version); } - - // Enqueue background warm-around - int warm_start = std::max(0, start - warm_margin); - int warm_end = std::min(max_rows - 1, end + warm_margin); - { - std::lock_guard lock(mtx_); - pending_.buf = &buf; - pending_.version = buf_version; - pending_.start_row = warm_start; - pending_.end_row = warm_end; - pending_.skip_first = start; - pending_.skip_last = end; - has_request_ = true; - } - ensure_worker_started(); - cv_.notify_one(); } } // namespace kte \ No newline at end of file diff --git a/syntax/HighlighterEngine.h b/syntax/HighlighterEngine.h index 70cb276..8e581a9 100644 --- a/syntax/HighlighterEngine.h +++ b/syntax/HighlighterEngine.h @@ -6,9 +6,6 @@ #include #include #include -#include -#include -#include #include "../Highlight.h" #include "LanguageHighlighter.h" @@ -39,9 +36,11 @@ public: } - // Phase 3: viewport-first prefetch and background warming - // Compute only the visible range now, and enqueue a background warm-around task. - // warm_margin: how many extra lines above/below to warm in the background. + // Compute highlights for the visible range so drawing hits the cache. + // There is deliberately no background warming: a worker thread would read + // the Buffer while the main thread edits it (and could outlive a moved + // Buffer), and the PieceTable is not safe for concurrent read and write. + // warm_margin is accepted for source compatibility and ignored. void PrefetchViewport(const Buffer &buf, int first_row, int row_count, std::uint64_t buf_version, int warm_margin = 200) const; @@ -62,29 +61,7 @@ private: // Track best known contiguous state row for a given version to avoid O(n) scans mutable std::unordered_map state_last_contig_; - // Thread-safety for caches and background worker state + // Guards the caches above. mutable std::mutex mtx_; - - // Background warmer - struct WarmRequest { - const Buffer *buf{nullptr}; - std::uint64_t version{0}; - int start_row{0}; - int end_row{0}; // inclusive - // Visible rows to skip touching in the background (inclusive range). - // These are computed synchronously by PrefetchViewport. - int skip_first{0}; - int skip_last{-1}; - }; - - mutable std::condition_variable cv_; - mutable std::thread worker_; - mutable std::atomic worker_running_{false}; - mutable bool has_request_{false}; - mutable WarmRequest pending_{}; - - void ensure_worker_started() const; - - void worker_loop() const; }; } // namespace kte