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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB
This commit is contained in:
@@ -1,26 +1,12 @@
|
||||
#include "HighlighterEngine.h"
|
||||
#include "../Buffer.h"
|
||||
#include "LanguageHighlighter.h"
|
||||
#include <thread>
|
||||
|
||||
namespace kte {
|
||||
HighlighterEngine::HighlighterEngine() = default;
|
||||
|
||||
|
||||
HighlighterEngine::~HighlighterEngine()
|
||||
{
|
||||
// stop background worker
|
||||
if (worker_running_.load()) {
|
||||
{
|
||||
std::lock_guard<std::mutex> 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<std::mutex> 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<std::mutex> 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
|
||||
@@ -6,9 +6,6 @@
|
||||
#include <unordered_map>
|
||||
#include <vector>
|
||||
#include <mutex>
|
||||
#include <condition_variable>
|
||||
#include <atomic>
|
||||
#include <thread>
|
||||
|
||||
#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<std::uint64_t, int> 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<bool> worker_running_{false};
|
||||
mutable bool has_request_{false};
|
||||
mutable WarmRequest pending_{};
|
||||
|
||||
void ensure_worker_started() const;
|
||||
|
||||
void worker_loop() const;
|
||||
};
|
||||
} // namespace kte
|
||||
|
||||
Reference in New Issue
Block a user