DragonX/ObsidianDragon
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 4 Wiki Activity

refactor(console): phase 3 — thread-safe ConsoleModel ingest queue; fix data race

Browse Source 
Introduce ConsoleModel (console_model.{h,cpp}): an ImGui-free, thread-safe line
store. Producers on any thread call ingest(), which only briefly locks a small
pending queue; the main thread calls drain() once per frame to move pending
lines into the visible deque and enforce the line cap, reporting how many were
added and evicted from the front.

This fixes the real data race. Previously a single lines_mutex_ guarded both the
line container AND the UI-only interaction state (selection, scroll, auto-scroll
counters), and it was held across the whole render pass — yet render() touched
auto_scroll_ / new_lines_since_scroll_ outside that lock, racing the background
RPC-trace and app-logger producers that call addLine() on worker threads.

Now:
- addLine() only ingests (lock-free w.r.t. the model); safe from any thread.
- render() drains once per frame, then does the cap/selection/scroll bookkeeping
  on the main thread (new drainModel()).
- The visible deque + all selection/scroll fields are main-thread-only, so every
  lines_mutex_ lock is gone — including the one wrapping the entire renderOutput.

Adds testConsoleModel() covering ingest/drain ordering, the line-cap eviction
counts, view-only clear (queued lines survive), and an 8-thread concurrent
ingest/drain stress with no lost lines. Full-node + lite build clean; ctest
green; source hygiene clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
DanS
2026-07-01 18:35:25 -05:00
parent 33735b1d52
commit 779c9682d4
6 changed files with 274 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files

107
src/ui/windows/console_tab.cpp
View File

@@ -208,6 +208,11 @@ void ConsoleTab::render(ConsoleCommandExecutor& exec)
while (exec.pollResult(result, isError)) addFormattedResult(result, isError);
}
// Drain the ingest queue into the visible model once per frame, after this frame's own
// ingests above and any that arrived from worker threads. From here on lines_ (via
// model_) and the selection/scroll state are touched only on this (main) thread.
drainModel();
// Main console layout
ImGui::BeginChild("ConsoleContainer", ImVec2(0, 0), false,
ImGuiWindowFlags_NoBackground | ImGuiWindowFlags_NoScrollbar);
@@ -458,7 +463,6 @@ void ConsoleTab::renderToolbar(ConsoleCommandExecutor& exec)
// Copy button — material styled
if (TactileButton(TR("copy"), ImVec2(0, 0), schema::UI().resolveFont("button"))) {
std::lock_guard<std::mutex> lock(lines_mutex_);
if (has_selection_) {
std::string selected = getSelectedText();
if (!selected.empty()) {
@@ -467,7 +471,7 @@ void ConsoleTab::renderToolbar(ConsoleCommandExecutor& exec)
} else {
// Copy all output if nothing selected
std::string all;
for (const auto& line : lines_) {
for (const auto& line : model_.lines()) {
all += line.text + "\n";
}
if (!all.empty()) {
@@ -493,10 +497,7 @@ void ConsoleTab::renderToolbar(ConsoleCommandExecutor& exec)
}
// Line count
{
std::lock_guard<std::mutex> lock(lines_mutex_);
ImGui::TextDisabled(TR("console_line_count"), lines_.size());
}
ImGui::TextDisabled(TR("console_line_count"), model_.size());
ImGui::SameLine();
ImGui::Spacing();
@@ -579,8 +580,9 @@ void ConsoleTab::renderOutput()
{
using namespace material;
auto& S = schema::UI();
std::lock_guard<std::mutex> lock(lines_mutex_);
// No lock: render() already drained the ingest queue this frame, and the visible model
// (model_) plus all selection/scroll state are touched only on this (main) thread.
// Zero item spacing so Dummy items advance the cursor by exactly their
// height. The inter-line gap is added explicitly to layout_.heights
// so that layout_.cumulativeY stays perfectly in sync with actual
@@ -617,13 +619,13 @@ void ConsoleTab::renderOutput()
outputFilter.errorsOnly;
visible_indices_.clear();
if (has_filter) {
for (int i = 0; i < static_cast<int>(lines_.size()); i++) {
if (!consoleLinePassesFilter(lines_[i].text, lines_[i].channel, outputFilter)) continue;
for (int i = 0; i < static_cast<int>(model_.size()); i++) {
if (!consoleLinePassesFilter(model_[i].text, model_[i].channel, outputFilter)) continue;
visible_indices_.push_back(i);
}
} else {
// No filter - all lines are visible
for (int i = 0; i < static_cast<int>(lines_.size()); i++) {
for (int i = 0; i < static_cast<int>(model_.size()); i++) {
visible_indices_.push_back(i);
}
}
@@ -642,7 +644,7 @@ void ConsoleTab::renderOutput()
ImFontConsoleMeasure measure(font, fontSize);
layout_ = BuildConsoleLayout(
visible_count,
[this](int vi) -> const std::string& { return lines_[visible_indices_[vi]].text; },
[this](int vi) -> const std::string& { return model_[visible_indices_[vi]].text; },
wrap_width, line_height, interLineGap, measure);
// Use raw IO for mouse handling to bypass child window event consumption
@@ -706,10 +708,10 @@ void ConsoleTab::renderOutput()
}
if (io.KeyCtrl && ImGui::IsKeyPressed(ImGuiKey_A)) {
// Select all
if (!lines_.empty()) {
if (!model_.empty()) {
sel_anchor_ = {0, 0};
sel_end_ = {static_cast<int>(lines_.size()) - 1,
static_cast<int>(lines_.back().text.size())};
sel_end_ = {static_cast<int>(model_.size()) - 1,
static_cast<int>(model_.back().text.size())};
has_selection_ = true;
}
}
@@ -761,7 +763,7 @@ void ConsoleTab::renderOutput()
last_rendered_vi = vi;
int i = visible_indices_[vi];
const auto& line = lines_[i];
const auto& line = model_[i];
const auto& segs = layout_.segments[vi];
ImVec2 lineOrigin = ImGui::GetCursorScreenPos();
float totalH = layout_.heights[vi];
@@ -914,7 +916,7 @@ void ConsoleTab::renderOutput()
if (has_text_filter) {
char filterBuf[128];
snprintf(filterBuf, sizeof(filterBuf), TR("console_showing_lines"),
visible_count, lines_.size());
visible_count, model_.size());
ImVec2 indicatorPos = ImGui::GetCursorScreenPos();
ImGui::GetWindowDrawList()->AddText(indicatorPos,
WithAlpha(Warning(), 180), filterBuf);
@@ -925,8 +927,8 @@ void ConsoleTab::renderOutput()
if (ImGui::IsMouseClicked(ImGuiMouseButton_Right)) {
TextPos tp = screenToTextPos(ImGui::GetMousePos());
context_token_.clear();
if (tp.line >= 0 && tp.line < static_cast<int>(lines_.size()))
context_token_ = extractCopyableToken(lines_[tp.line].text, tp.col);
if (tp.line >= 0 && tp.line < static_cast<int>(model_.size()))
context_token_ = extractCopyableToken(model_[tp.line].text, tp.col);
}
// Right-click context menu
@@ -948,17 +950,17 @@ void ConsoleTab::renderOutput()
}
}
if (ImGui::MenuItem(TR("console_select_all"), "Ctrl+A")) {
if (!lines_.empty()) {
if (!model_.empty()) {
sel_anchor_ = {0, 0};
sel_end_ = {static_cast<int>(lines_.size()) - 1,
static_cast<int>(lines_.back().text.size())};
sel_end_ = {static_cast<int>(model_.size()) - 1,
static_cast<int>(model_.back().text.size())};
has_selection_ = true;
}
}
ImGui::Separator();
if (ImGui::MenuItem(TR("console_clear_console"))) {
// Can't call clear() here (already holding lock), just mark for clearing
lines_.clear();
// View-only clear (main thread) — drop the visible lines and the selection.
model_.clear();
has_selection_ = false;
}
ImGui::EndPopup();
@@ -1013,7 +1015,7 @@ ConsoleTab::TextPos ConsoleTab::screenToTextPos(ImVec2 screen_pos) const
ImFontConsoleMeasure measure(ImGui::GetFont(), ImGui::GetFontSize());
ConsoleHit hit = HitTestConsoleLayout(
layout_, static_cast<int>(visible_indices_.size()),
[this](int vi) -> const std::string& { return lines_[visible_indices_[vi]].text; },
[this](int vi) -> const std::string& { return model_[visible_indices_[vi]].text; },
screen_pos.x - output_origin_.x, screen_pos.y - output_origin_.y, measure);
TextPos pos;
@@ -1045,8 +1047,8 @@ std::string ConsoleTab::getSelectedText() const
TextPos start = selectionStart();
TextPos end = selectionEnd();
return ExtractConsoleSelection(
static_cast<int>(lines_.size()),
[this](int i) -> const std::string& { return lines_[i].text; },
static_cast<int>(model_.size()),
[this](int i) -> const std::string& { return model_[i].text; },
visible_indices_,
{start.line, start.col}, {end.line, end.col});
}
@@ -1408,37 +1410,39 @@ void ConsoleTab::renderStatusHeader(ConsoleCommandExecutor& exec)
void ConsoleTab::addLine(const std::string& line, ConsoleChannel channel)
{
std::lock_guard<std::mutex> lock(lines_mutex_);
// The channel is the canonical semantic key — text color and the left accent bar are
// both derived from it at draw time (channelTextColor / the bar switch). Sources emit
// clean text (no "[daemon]"/"[xmrig]"/"[app]"/"[rpc]" prefix); the bar carries origin.
lines_.push_back({line, channel});
//
// ingest() is thread-safe, so this is callable from the RPC-trace / app-logger worker
// threads. The line becomes visible on the next render() drain (main thread), where the
// line-cap eviction and selection/scroll bookkeeping happen — none of the UI-only state
// is touched here, so there is no cross-thread access to it.
model_.ingest(line, channel);
}
// Limit buffer size — adjust selection indices when lines are removed
// from the front so the highlight stays on the text the user selected.
int popped = 0;
while (lines_.size() > 10000) {
lines_.pop_front();
popped++;
}
if (popped > 0 && has_selection_) {
sel_anchor_.line -= popped;
sel_end_.line -= popped;
void ConsoleTab::drainModel()
{
const ConsoleModel::DrainResult dr = model_.drain();
if (dr.added == 0) return;
// Lines evicted from the front by the cap shift every index down — move the selection
// with them so the highlight stays on the text the user selected.
if (dr.popped > 0 && has_selection_) {
sel_anchor_.line -= static_cast<int>(dr.popped);
sel_end_.line -= static_cast<int>(dr.popped);
if (sel_anchor_.line < 0 && sel_end_.line < 0) {
// Entire selection was in the removed range
has_selection_ = false;
has_selection_ = false; // entire selection was in the removed range
is_selecting_ = false;
} else {
if (sel_anchor_.line < 0) { sel_anchor_.line = 0; sel_anchor_.col = 0; }
if (sel_end_.line < 0) { sel_end_.line = 0; sel_end_.col = 0; }
}
}
// Track new output while user is scrolled up
if (!auto_scroll_) {
new_lines_since_scroll_++;
}
// Track new output that arrived while the user is scrolled up (for the indicator).
if (!auto_scroll_)
new_lines_since_scroll_ += static_cast<int>(dr.added);
}
void ConsoleTab::addRpcTraceLine(const std::string& source, const std::string& method)
@@ -1449,12 +1453,9 @@ void ConsoleTab::addRpcTraceLine(const std::string& source, const std::string& m
void ConsoleTab::clear()
{
{
std::lock_guard<std::mutex> lock(lines_mutex_);
lines_.clear();
}
// The executor keeps its own log cursors, so a view-clear stays cleared (new output
// still appends). addLine() takes the lock itself, so call it outside the locked scope.
// View-only clear (main thread). The executor keeps its own log cursors, so new output
// still appends. The "cleared" line is ingested and appears on the next frame's drain.
model_.clear();
addLine(TR("console_cleared"), ConsoleChannel::Info);
}