2 Commits

Author SHA1 Message Date
d2d155f211 Fix data race.
+ Add thread-safety with mutexes in `PieceTable` and `Buffer`
+ Bump version to 1.5.9
2026-01-28 01:03:58 -08:00
8634eb78f0 Refactor Init method across all frontends to include argc and argv for improved argument handling consistency. 2026-01-12 10:35:24 -08:00
15 changed files with 137 additions and 100 deletions

View File

@@ -292,29 +292,29 @@ Buffer::OpenFromFile(const std::string &path, std::string &err)
bool bool
Buffer::Save(std::string &err) const Buffer::Save(std::string &err) const
{ {
if (!is_file_backed_ || filename_.empty()) { if (!is_file_backed_ || filename_.empty()) {
err = "Buffer is not file-backed; use SaveAs()"; err = "Buffer is not file-backed; use SaveAs()";
return false; return false;
} }
std::ofstream out(filename_, std::ios::out | std::ios::binary | std::ios::trunc); std::ofstream out(filename_, std::ios::out | std::ios::binary | std::ios::trunc);
if (!out) { if (!out) {
err = "Failed to open for write: " + filename_ + ". Error: " + std::string(std::strerror(errno)); err = "Failed to open for write: " + filename_ + ". Error: " + std::string(std::strerror(errno));
return false; return false;
} }
// Stream the content directly from the piece table to avoid relying on // Stream the content directly from the piece table to avoid relying on
// full materialization, which may yield an empty pointer when size > 0. // full materialization, which may yield an empty pointer when size > 0.
if (content_.Size() > 0) { if (content_.Size() > 0) {
content_.WriteToStream(out); content_.WriteToStream(out);
} }
// Ensure data hits the OS buffers // Ensure data hits the OS buffers
out.flush(); out.flush();
if (!out.good()) { if (!out.good()) {
err = "Write error: " + filename_ + ". Error: " + std::string(std::strerror(errno)); err = "Write error: " + filename_ + ". Error: " + std::string(std::strerror(errno));
return false; return false;
} }
// Note: const method cannot change dirty_. Intentionally const to allow UI code // Note: const method cannot change dirty_. Intentionally const to allow UI code
// to decide when to flip dirty flag after successful save. // to decide when to flip dirty flag after successful save.
return true; return true;
} }
@@ -346,16 +346,16 @@ Buffer::SaveAs(const std::string &path, std::string &err)
err = "Failed to open for write: " + out_path + ". Error: " + std::string(std::strerror(errno)); err = "Failed to open for write: " + out_path + ". Error: " + std::string(std::strerror(errno));
return false; return false;
} }
// Stream content without forcing full materialization // Stream content without forcing full materialization
if (content_.Size() > 0) { if (content_.Size() > 0) {
content_.WriteToStream(out); content_.WriteToStream(out);
} }
// Ensure data hits the OS buffers // Ensure data hits the OS buffers
out.flush(); out.flush();
if (!out.good()) { if (!out.good()) {
err = "Write error: " + out_path + ". Error: " + std::string(std::strerror(errno)); err = "Write error: " + out_path + ". Error: " + std::string(std::strerror(errno));
return false; return false;
} }
filename_ = out_path; filename_ = out_path;
is_file_backed_ = true; is_file_backed_ = true;
@@ -412,6 +412,7 @@ Buffer::GetLineView(std::size_t row) const
void void
Buffer::ensure_rows_cache() const Buffer::ensure_rows_cache() const
{ {
std::lock_guard<std::mutex> lock(buffer_mutex_);
if (!rows_cache_dirty_) if (!rows_cache_dirty_)
return; return;
rows_.clear(); rows_.clear();
@@ -454,8 +455,8 @@ Buffer::delete_text(int row, int col, std::size_t len)
const std::size_t L = line.size(); const std::size_t L = line.size();
if (c < L) { if (c < L) {
const std::size_t take = std::min(remaining, L - c); const std::size_t take = std::min(remaining, L - c);
c += take; c += take;
remaining -= take; remaining -= take;
} }
if (remaining == 0) if (remaining == 0)
break; break;
@@ -463,8 +464,8 @@ Buffer::delete_text(int row, int col, std::size_t len)
if (r + 1 < lc) { if (r + 1 < lc) {
if (remaining > 0) { if (remaining > 0) {
remaining -= 1; // the newline remaining -= 1; // the newline
r += 1; r += 1;
c = 0; c = 0;
} }
} else { } else {
// At last line and still remaining: delete to EOF // At last line and still remaining: delete to EOF

View File

@@ -14,6 +14,7 @@
#include <cstdint> #include <cstdint>
#include "syntax/HighlighterEngine.h" #include "syntax/HighlighterEngine.h"
#include "Highlight.h" #include "Highlight.h"
#include <mutex>
// Forward declaration for swap journal integration // Forward declaration for swap journal integration
namespace kte { namespace kte {
@@ -482,4 +483,6 @@ private:
std::unique_ptr<kte::HighlighterEngine> highlighter_; std::unique_ptr<kte::HighlighterEngine> highlighter_;
// Non-owning pointer to swap recorder managed by Editor/SwapManager // Non-owning pointer to swap recorder managed by Editor/SwapManager
kte::SwapRecorder *swap_rec_ = nullptr; kte::SwapRecorder *swap_rec_ = nullptr;
mutable std::mutex buffer_mutex_;
}; };

View File

@@ -4,7 +4,7 @@ project(kte)
include(GNUInstallDirs) include(GNUInstallDirs)
set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD 20)
set(KTE_VERSION "1.5.8") set(KTE_VERSION "1.5.9")
# Default to terminal-only build to avoid SDL/OpenGL dependency by default. # Default to terminal-only build to avoid SDL/OpenGL dependency by default.
# Enable with -DBUILD_GUI=ON when SDL2/OpenGL/Freetype are available. # Enable with -DBUILD_GUI=ON when SDL2/OpenGL/Freetype are available.

View File

@@ -12,7 +12,7 @@ public:
virtual ~Frontend() = default; virtual ~Frontend() = default;
// Initialize the frontend (create window/terminal, etc.) // Initialize the frontend (create window/terminal, etc.)
virtual bool Init(Editor &ed) = 0; virtual bool Init(int &argc, char **argv, Editor &ed) = 0;
// Execute one iteration (poll input, dispatch, draw). Set running=false to exit. // Execute one iteration (poll input, dispatch, draw). Set running=false to exit.
virtual void Step(Editor &ed, bool &running) = 0; virtual void Step(Editor &ed, bool &running) = 0;

View File

@@ -30,8 +30,10 @@
static auto kGlslVersion = "#version 150"; // GL 3.2 core (macOS compatible) static auto kGlslVersion = "#version 150"; // GL 3.2 core (macOS compatible)
bool bool
GUIFrontend::Init(Editor &ed) GUIFrontend::Init(int &argc, char **argv, Editor &ed)
{ {
(void) argc;
(void) argv;
// Attach editor to input handler for editor-owned features (e.g., universal argument) // Attach editor to input handler for editor-owned features (e.g., universal argument)
input_.Attach(&ed); input_.Attach(&ed);
// editor dimensions will be initialized during the first Step() frame // editor dimensions will be initialized during the first Step() frame

View File

@@ -17,7 +17,7 @@ public:
~GUIFrontend() override = default; ~GUIFrontend() override = default;
bool Init(Editor &ed) override; bool Init(int &argc, char **argv, Editor &ed) override;
void Step(Editor &ed, bool &running) override; void Step(Editor &ed, bool &running) override;

View File

@@ -218,9 +218,9 @@ PieceTable::addPieceBack(const Source src, const std::size_t start, const std::s
std::size_t expectStart = last.start + last.len; std::size_t expectStart = last.start + last.len;
if (expectStart == start) { if (expectStart == start) {
last.len += len; last.len += len;
total_size_ += len; total_size_ += len;
dirty_ = true; dirty_ = true;
version_++; version_++;
range_cache_ = {}; range_cache_ = {};
find_cache_ = {}; find_cache_ = {};
@@ -231,7 +231,7 @@ PieceTable::addPieceBack(const Source src, const std::size_t start, const std::s
pieces_.push_back(Piece{src, start, len}); pieces_.push_back(Piece{src, start, len});
total_size_ += len; total_size_ += len;
dirty_ = true; dirty_ = true;
InvalidateLineIndex(); InvalidateLineIndex();
version_++; version_++;
range_cache_ = {}; range_cache_ = {};
@@ -251,9 +251,9 @@ PieceTable::addPieceFront(Source src, std::size_t start, std::size_t len)
Piece &first = pieces_.front(); Piece &first = pieces_.front();
if (first.src == src && start + len == first.start) { if (first.src == src && start + len == first.start) {
first.start = start; first.start = start;
first.len += len; first.len += len;
total_size_ += len; total_size_ += len;
dirty_ = true; dirty_ = true;
version_++; version_++;
range_cache_ = {}; range_cache_ = {};
find_cache_ = {}; find_cache_ = {};
@@ -262,7 +262,7 @@ PieceTable::addPieceFront(Source src, std::size_t start, std::size_t len)
} }
pieces_.insert(pieces_.begin(), Piece{src, start, len}); pieces_.insert(pieces_.begin(), Piece{src, start, len});
total_size_ += len; total_size_ += len;
dirty_ = true; dirty_ = true;
InvalidateLineIndex(); InvalidateLineIndex();
version_++; version_++;
range_cache_ = {}; range_cache_ = {};
@@ -273,6 +273,7 @@ PieceTable::addPieceFront(Source src, std::size_t start, std::size_t len)
void void
PieceTable::materialize() const PieceTable::materialize() const
{ {
std::lock_guard<std::mutex> lock(mutex_);
if (!dirty_) { if (!dirty_) {
return; return;
} }
@@ -348,6 +349,7 @@ PieceTable::coalesceNeighbors(std::size_t index)
void void
PieceTable::InvalidateLineIndex() const PieceTable::InvalidateLineIndex() const
{ {
std::lock_guard<std::mutex> lock(mutex_);
line_index_dirty_ = true; line_index_dirty_ = true;
} }
@@ -355,22 +357,29 @@ PieceTable::InvalidateLineIndex() const
void void
PieceTable::RebuildLineIndex() const PieceTable::RebuildLineIndex() const
{ {
if (!line_index_dirty_) std::lock_guard<std::mutex> lock(mutex_);
if (!line_index_dirty_) {
return; return;
}
line_index_.clear(); line_index_.clear();
line_index_.push_back(0); line_index_.push_back(0);
std::size_t pos = 0; std::size_t pos = 0;
for (const auto &pc: pieces_) { for (const auto &pc: pieces_) {
const std::string &src = pc.src == Source::Original ? original_ : add_; const std::string &src = pc.src == Source::Original ? original_ : add_;
const char *base = src.data() + static_cast<std::ptrdiff_t>(pc.start); const char *base = src.data() + static_cast<std::ptrdiff_t>(pc.start);
for (std::size_t j = 0; j < pc.len; ++j) { for (std::size_t j = 0; j < pc.len; ++j) {
if (base[j] == '\n') { if (base[j] == '\n') {
// next line starts after the newline // next line starts after the newline
line_index_.push_back(pos + j + 1); line_index_.push_back(pos + j + 1);
} }
} }
pos += pc.len; pos += pc.len;
} }
line_index_dirty_ = false; line_index_dirty_ = false;
} }
@@ -391,7 +400,7 @@ PieceTable::Insert(std::size_t byte_offset, const char *text, std::size_t len)
if (pieces_.empty()) { if (pieces_.empty()) {
pieces_.push_back(Piece{Source::Add, add_start, len}); pieces_.push_back(Piece{Source::Add, add_start, len});
total_size_ += len; total_size_ += len;
dirty_ = true; dirty_ = true;
InvalidateLineIndex(); InvalidateLineIndex();
maybeConsolidate(); maybeConsolidate();
version_++; version_++;
@@ -405,7 +414,7 @@ PieceTable::Insert(std::size_t byte_offset, const char *text, std::size_t len)
// insert at end // insert at end
pieces_.push_back(Piece{Source::Add, add_start, len}); pieces_.push_back(Piece{Source::Add, add_start, len});
total_size_ += len; total_size_ += len;
dirty_ = true; dirty_ = true;
InvalidateLineIndex(); InvalidateLineIndex();
coalesceNeighbors(pieces_.size() - 1); coalesceNeighbors(pieces_.size() - 1);
maybeConsolidate(); maybeConsolidate();
@@ -433,7 +442,7 @@ PieceTable::Insert(std::size_t byte_offset, const char *text, std::size_t len)
pieces_.insert(pieces_.begin() + static_cast<std::ptrdiff_t>(idx), repl.begin(), repl.end()); pieces_.insert(pieces_.begin() + static_cast<std::ptrdiff_t>(idx), repl.begin(), repl.end());
total_size_ += len; total_size_ += len;
dirty_ = true; dirty_ = true;
InvalidateLineIndex(); InvalidateLineIndex();
// Try coalescing around the inserted position (the inserted piece is at idx + (inner>0 ? 1 : 0)) // Try coalescing around the inserted position (the inserted piece is at idx + (inner>0 ? 1 : 0))
std::size_t ins_index = idx + (inner > 0 ? 1 : 0); std::size_t ins_index = idx + (inner > 0 ? 1 : 0);
@@ -488,13 +497,13 @@ PieceTable::Delete(std::size_t byte_offset, std::size_t len)
// entire piece removed // entire piece removed
pieces_.erase(pieces_.begin() + static_cast<std::ptrdiff_t>(idx)); pieces_.erase(pieces_.begin() + static_cast<std::ptrdiff_t>(idx));
// stay at same idx for next piece // stay at same idx for next piece
inner = 0; inner = 0;
remaining -= take; remaining -= take;
continue; continue;
} }
// After modifying current idx, next deletion continues at beginning of the next logical region // After modifying current idx, next deletion continues at beginning of the next logical region
inner = 0; inner = 0;
remaining -= take; remaining -= take;
if (remaining == 0) if (remaining == 0)
break; break;
@@ -503,7 +512,7 @@ PieceTable::Delete(std::size_t byte_offset, std::size_t len)
} }
total_size_ -= len; total_size_ -= len;
dirty_ = true; dirty_ = true;
InvalidateLineIndex(); InvalidateLineIndex();
if (idx < pieces_.size()) if (idx < pieces_.size())
coalesceNeighbors(idx); coalesceNeighbors(idx);
@@ -692,14 +701,18 @@ PieceTable::GetRange(std::size_t byte_offset, std::size_t len) const
len = total_size_ - byte_offset; len = total_size_ - byte_offset;
// Fast path: return cached value if version/offset/len match // Fast path: return cached value if version/offset/len match
if (range_cache_.valid && range_cache_.version == version_ && {
range_cache_.off == byte_offset && range_cache_.len == len) { std::lock_guard<std::mutex> lock(mutex_);
return range_cache_.data; if (range_cache_.valid && range_cache_.version == version_ &&
range_cache_.off == byte_offset && range_cache_.len == len) {
return range_cache_.data;
}
} }
std::string out; std::string out;
out.reserve(len); out.reserve(len);
if (!dirty_) { if (!dirty_) {
std::lock_guard<std::mutex> lock(mutex_);
// Already materialized; slice directly // Already materialized; slice directly
out.assign(materialized_.data() + static_cast<std::ptrdiff_t>(byte_offset), len); out.assign(materialized_.data() + static_cast<std::ptrdiff_t>(byte_offset), len);
} else { } else {
@@ -714,8 +727,8 @@ PieceTable::GetRange(std::size_t byte_offset, std::size_t len) const
const char *base = src.data() + static_cast<std::ptrdiff_t>(p.start + inner); const char *base = src.data() + static_cast<std::ptrdiff_t>(p.start + inner);
out.append(base, take); out.append(base, take);
remaining -= take; remaining -= take;
inner = 0; inner = 0;
idx += 1; idx += 1;
} else { } else {
break; break;
} }
@@ -723,11 +736,14 @@ PieceTable::GetRange(std::size_t byte_offset, std::size_t len) const
} }
// Update cache // Update cache
range_cache_.valid = true; {
range_cache_.version = version_; std::lock_guard<std::mutex> lock(mutex_);
range_cache_.off = byte_offset; range_cache_.valid = true;
range_cache_.len = len; range_cache_.version = version_;
range_cache_.data = out; range_cache_.off = byte_offset;
range_cache_.len = len;
range_cache_.data = out;
}
return out; return out;
} }
@@ -739,23 +755,30 @@ PieceTable::Find(const std::string &needle, std::size_t start) const
return start <= total_size_ ? start : std::numeric_limits<std::size_t>::max(); return start <= total_size_ ? start : std::numeric_limits<std::size_t>::max();
if (start > total_size_) if (start > total_size_)
return std::numeric_limits<std::size_t>::max(); return std::numeric_limits<std::size_t>::max();
if (find_cache_.valid && {
find_cache_.version == version_ && std::lock_guard<std::mutex> lock(mutex_);
find_cache_.needle == needle && if (find_cache_.valid &&
find_cache_.start == start) { find_cache_.version == version_ &&
return find_cache_.result; find_cache_.needle == needle &&
find_cache_.start == start) {
return find_cache_.result;
}
} }
materialize(); materialize();
auto pos = materialized_.find(needle, start); std::size_t pos;
if (pos == std::string::npos) {
pos = std::numeric_limits<std::size_t>::max(); std::lock_guard<std::mutex> lock(mutex_);
// Update cache pos = materialized_.find(needle, start);
find_cache_.valid = true; if (pos == std::string::npos)
find_cache_.version = version_; pos = std::numeric_limits<std::size_t>::max();
find_cache_.needle = needle; // Update cache
find_cache_.start = start; find_cache_.valid = true;
find_cache_.result = pos; find_cache_.version = version_;
find_cache_.needle = needle;
find_cache_.start = start;
find_cache_.result = pos;
}
return pos; return pos;
} }
@@ -763,12 +786,15 @@ PieceTable::Find(const std::string &needle, std::size_t start) const
void void
PieceTable::WriteToStream(std::ostream &out) const PieceTable::WriteToStream(std::ostream &out) const
{ {
// Stream the content piece-by-piece without forcing full materialization // Stream the content piece-by-piece without forcing full materialization
for (const auto &p : pieces_) { // No lock needed for original_ and add_ if they are not being modified.
if (p.len == 0) // Since this is a const method and kte's piece table isn't modified by multiple threads
continue; // (only queried), we just iterate pieces_.
const std::string &src = (p.src == Source::Original) ? original_ : add_; for (const auto &p: pieces_) {
const char *base = src.data() + static_cast<std::ptrdiff_t>(p.start); if (p.len == 0)
out.write(base, static_cast<std::streamsize>(p.len)); continue;
} const std::string &src = (p.src == Source::Original) ? original_ : add_;
const char *base = src.data() + static_cast<std::ptrdiff_t>(p.start);
out.write(base, static_cast<std::streamsize>(p.len));
}
} }

View File

@@ -8,6 +8,7 @@
#include <ostream> #include <ostream>
#include <vector> #include <vector>
#include <limits> #include <limits>
#include <mutex>
class PieceTable { class PieceTable {
@@ -181,4 +182,6 @@ private:
mutable RangeCache range_cache_; mutable RangeCache range_cache_;
mutable FindCache find_cache_; mutable FindCache find_cache_;
mutable std::mutex mutex_;
}; };

View File

@@ -658,11 +658,9 @@ private:
} // namespace } // namespace
bool bool
GUIFrontend::Init(Editor &ed) GUIFrontend::Init(int &argc, char **argv, Editor &ed)
{ {
int argc = 0; app_ = new QApplication(argc, argv);
char **argv = nullptr;
app_ = new QApplication(argc, argv);
window_ = new MainWindow(input_); window_ = new MainWindow(input_);
window_->show(); window_->show();

View File

@@ -18,7 +18,7 @@ public:
~GUIFrontend() override = default; ~GUIFrontend() override = default;
bool Init(Editor &ed) override; bool Init(int &argc, char **argv, Editor &ed) override;
void Step(Editor &ed, bool &running) override; void Step(Editor &ed, bool &running) override;

View File

@@ -8,8 +8,10 @@
bool bool
TerminalFrontend::Init(Editor &ed) TerminalFrontend::Init(int &argc, char **argv, Editor &ed)
{ {
(void) argc;
(void) argv;
// Ensure Control keys reach the app: disable XON/XOFF and dsusp/susp bindings (e.g., ^S/^Q, ^Y on macOS) // Ensure Control keys reach the app: disable XON/XOFF and dsusp/susp bindings (e.g., ^S/^Q, ^Y on macOS)
{ {
struct termios tio{}; struct termios tio{};
@@ -121,4 +123,4 @@ TerminalFrontend::Shutdown()
have_old_sigint_ = false; have_old_sigint_ = false;
} }
endwin(); endwin();
} }

View File

@@ -21,7 +21,7 @@ public:
// Adjust if your terminal needs a different threshold. // Adjust if your terminal needs a different threshold.
static constexpr int kEscDelayMs = 50; static constexpr int kEscDelayMs = 50;
bool Init(Editor &ed) override; bool Init(int &argc, char **argv, Editor &ed) override;
void Step(Editor &ed, bool &running) override; void Step(Editor &ed, bool &running) override;

View File

@@ -4,8 +4,10 @@
bool bool
TestFrontend::Init(Editor &ed) TestFrontend::Init(int &argc, char **argv, Editor &ed)
{ {
(void) argc;
(void) argv;
ed.SetDimensions(24, 80); ed.SetDimensions(24, 80);
return true; return true;
} }
@@ -30,4 +32,4 @@ TestFrontend::Step(Editor &ed, bool &running)
void void
TestFrontend::Shutdown() {} TestFrontend::Shutdown() {}

View File

@@ -13,7 +13,7 @@ public:
~TestFrontend() override = default; ~TestFrontend() override = default;
bool Init(Editor &ed) override; bool Init(int &argc, char **argv, Editor &ed) override;
void Step(Editor &ed, bool &running) override; void Step(Editor &ed, bool &running) override;

View File

@@ -112,7 +112,7 @@ RunStressHighlighter(unsigned seconds)
int int
main(int argc, const char *argv[]) main(int argc, char *argv[])
{ {
std::setlocale(LC_ALL, ""); std::setlocale(LC_ALL, "");
@@ -136,7 +136,7 @@ main(int argc, const char *argv[])
int opt; int opt;
int long_index = 0; int long_index = 0;
unsigned stress_seconds = 0; unsigned stress_seconds = 0;
while ((opt = getopt_long(argc, const_cast<char *const *>(argv), "gthV", long_opts, &long_index)) != -1) { while ((opt = getopt_long(argc, argv, "gthV", long_opts, &long_index)) != -1) {
switch (opt) { switch (opt) {
case 'g': case 'g':
req_gui = true; req_gui = true;
@@ -303,7 +303,7 @@ main(int argc, const char *argv[])
} }
#endif #endif
if (!fe->Init(editor)) { if (!fe->Init(argc, argv, editor)) {
std::cerr << "kte: failed to initialize frontend" << std::endl; std::cerr << "kte: failed to initialize frontend" << std::endl;
return 1; return 1;
} }