Skip to content

Commit ded8ef3

Browse files
committed
qml: avoid QtConcurrent in debug log model
1 parent 22ed52f commit ded8ef3

3 files changed

Lines changed: 154 additions & 49 deletions

File tree

qml/bitcoin.cpp

Lines changed: 34 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
#include <tuple>
6666

6767
#include <QDebug>
68+
#include <QCoreApplication>
6869
#include <QFontDatabase>
6970
#include <QGuiApplication>
7071
#include <QQmlApplicationEngine>
@@ -341,6 +342,7 @@ int QmlGuiMain(int argc, char* argv[])
341342
node_model.addStartupWarnings(startup_warnings);
342343
QmlInitExecutor init_executor{*node};
343344
bool shutdown_requested{false};
345+
DebugLogModel debug_log_model{gArgs.GetDataDirNet() / "debug.log"};
344346
#ifdef ENABLE_WALLET
345347
std::unique_ptr<WalletQmlController> wallet_controller;
346348
if (wallet_enabled) {
@@ -365,11 +367,12 @@ int QmlGuiMain(int argc, char* argv[])
365367
wallet_controller->unloadWallets();
366368
}
367369
#endif
368-
node->startShutdown();
369370
init_executor.shutdown();
370371
});
371372
QObject::connect(&init_executor, &QmlInitExecutor::initializeResult, &node_model, &NodeModel::initializeResult);
372-
QObject::connect(&init_executor, &QmlInitExecutor::shutdownResult, qGuiApp, &QGuiApplication::quit, Qt::QueuedConnection);
373+
QObject::connect(&init_executor, &QmlInitExecutor::shutdownResult, qGuiApp, [] {
374+
QCoreApplication::exit(0);
375+
}, Qt::QueuedConnection);
373376
QObject::connect(&init_executor, &QmlInitExecutor::runawayException, &node_model, &NodeModel::handleRunawayException);
374377

375378
NetworkTrafficTower network_traffic_tower{node_model};
@@ -405,23 +408,21 @@ int QmlGuiMain(int argc, char* argv[])
405408
LoadFontResource(":/fonts/bitcoincoresans/semibold");
406409
LoadFontResource(":/fonts/robotomono/regular");
407410

408-
QQmlApplicationEngine engine;
411+
auto engine = std::make_unique<QQmlApplicationEngine>();
409412

410413
QScopedPointer<const NetworkStyle> network_style{NetworkStyle::instantiate(Params().GetChainType())};
411414
assert(!network_style.isNull());
412-
engine.addImageProvider(QStringLiteral("images"), new ImageProvider{network_style.data()});
413-
engine.addImageProvider(QStringLiteral("qr"), new QRImageProvider);
414-
415-
engine.rootContext()->setContextProperty("networkTrafficTower", &network_traffic_tower);
416-
engine.rootContext()->setContextProperty("networkStatusModel", &network_status_model);
417-
engine.rootContext()->setContextProperty("nodeModel", &node_model);
418-
engine.rootContext()->setContextProperty("chainModel", &chain_model);
419-
engine.rootContext()->setContextProperty("peerTableModel", &peer_model);
420-
engine.rootContext()->setContextProperty("peerListModelProxy", &peer_model_sort_proxy);
421-
engine.rootContext()->setContextProperty("banListModel", &ban_list_model);
422-
423-
DebugLogModel debug_log_model{gArgs.GetDataDirNet() / "debug.log"};
424-
engine.rootContext()->setContextProperty("debugLogModel", &debug_log_model);
415+
engine->addImageProvider(QStringLiteral("images"), new ImageProvider{network_style.data()});
416+
engine->addImageProvider(QStringLiteral("qr"), new QRImageProvider);
417+
418+
engine->rootContext()->setContextProperty("networkTrafficTower", &network_traffic_tower);
419+
engine->rootContext()->setContextProperty("networkStatusModel", &network_status_model);
420+
engine->rootContext()->setContextProperty("nodeModel", &node_model);
421+
engine->rootContext()->setContextProperty("chainModel", &chain_model);
422+
engine->rootContext()->setContextProperty("peerTableModel", &peer_model);
423+
engine->rootContext()->setContextProperty("peerListModelProxy", &peer_model_sort_proxy);
424+
engine->rootContext()->setContextProperty("banListModel", &ban_list_model);
425+
engine->rootContext()->setContextProperty("debugLogModel", &debug_log_model);
425426

426427
#ifdef ENABLE_WALLET
427428
std::unique_ptr<WalletListModel> wallet_list_model;
@@ -449,18 +450,18 @@ int QmlGuiMain(int argc, char* argv[])
449450
list_model->listWalletDir();
450451
}
451452
});
452-
engine.rootContext()->setContextProperty("walletController", wallet_controller.get());
453-
engine.rootContext()->setContextProperty("walletListModel", wallet_list_model.get());
453+
engine->rootContext()->setContextProperty("walletController", wallet_controller.get());
454+
engine->rootContext()->setContextProperty("walletListModel", wallet_list_model.get());
454455
}
455456
#endif
456457

457458
OptionsQmlModel options_model(*node, !need_onboarding.toBool());
458-
engine.rootContext()->setContextProperty("optionsModel", &options_model);
459-
engine.rootContext()->setContextProperty("needOnboarding", need_onboarding);
459+
engine->rootContext()->setContextProperty("optionsModel", &options_model);
460+
engine->rootContext()->setContextProperty("needOnboarding", need_onboarding);
460461
#ifdef ENABLE_TEST_AUTOMATION
461-
engine.rootContext()->setContextProperty("testAutomationEnabled", true);
462+
engine->rootContext()->setContextProperty("testAutomationEnabled", true);
462463
#else
463-
engine.rootContext()->setContextProperty("testAutomationEnabled", false);
464+
engine->rootContext()->setContextProperty("testAutomationEnabled", false);
464465
#endif
465466

466467
// -lang CLI flag overrides the persisted setting (bitcoin-qt compatibility).
@@ -481,7 +482,7 @@ int QmlGuiMain(int argc, char* argv[])
481482
// Retranslate the QML UI immediately when the user picks a new language.
482483
QObject::connect(&options_model, &OptionsQmlModel::languageChanged, [&]() {
483484
install_language(options_model.language());
484-
engine.retranslate();
485+
engine->retranslate();
485486
});
486487

487488
BuildInfo build_info;
@@ -515,12 +516,12 @@ int QmlGuiMain(int argc, char* argv[])
515516
"WalletListModel cannot be instantiated from QML");
516517
#endif
517518

518-
engine.load(QUrl(QStringLiteral("qrc:///qml/pages/main.qml")));
519-
if (engine.rootObjects().isEmpty()) {
519+
engine->load(QUrl(QStringLiteral("qrc:///qml/pages/main.qml")));
520+
if (engine->rootObjects().isEmpty()) {
520521
return EXIT_FAILURE;
521522
}
522523

523-
auto window = qobject_cast<QQuickWindow*>(engine.rootObjects().first());
524+
auto window = qobject_cast<QQuickWindow*>(engine->rootObjects().first());
524525
if (!window) {
525526
return EXIT_FAILURE;
526527
}
@@ -534,7 +535,7 @@ int QmlGuiMain(int argc, char* argv[])
534535
socket_path = QString::fromStdString(
535536
(gArgs.GetDataDirNet() / "test_bridge.sock").utf8string());
536537
}
537-
test_bridge = std::make_unique<TestBridge>(&engine, socket_path);
538+
test_bridge = std::make_unique<TestBridge>(engine.get(), socket_path);
538539
}
539540
#endif
540541

@@ -544,5 +545,10 @@ int QmlGuiMain(int argc, char* argv[])
544545
qInfo() << "Graphics API in use:" << QmlUtil::GraphicsApi(window);
545546

546547
node_model.startShutdownPolling();
547-
return qGuiApp->exec();
548+
const int exit_code{qGuiApp->exec()};
549+
#ifdef ENABLE_TEST_AUTOMATION
550+
test_bridge.reset();
551+
#endif
552+
engine.reset();
553+
return exit_code;
548554
}

qml/models/debuglogmodel.cpp

Lines changed: 102 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,21 @@
44

55
#include <qml/models/debuglogmodel.h>
66

7+
#include <util/threadnames.h>
8+
79
#include <algorithm>
10+
#include <utility>
811

912
#include <QDateTime>
1013
#include <QDesktopServices>
1114
#include <QFile>
12-
#include <QFutureWatcher>
15+
#include <QMetaObject>
16+
#include <QObject>
1317
#include <QRegularExpression>
1418
#include <QTextStream>
19+
#include <QThread>
20+
#include <QTimer>
1521
#include <QUrl>
16-
#include <QtConcurrent/QtConcurrentRun>
1722

1823
static const QRegularExpression TIMESTAMP_RX(
1924
QStringLiteral(R"(^(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?Z)\s*(.*)$)"));
@@ -22,13 +27,30 @@ DebugLogModel::DebugLogModel(const fs::path& log_path, QObject* parent)
2227
: QAbstractListModel(parent)
2328
, m_log_path(log_path)
2429
{
30+
m_reader = new QObject;
31+
m_reader_thread = new QThread(this);
32+
m_reader->moveToThread(m_reader_thread);
33+
connect(m_reader_thread, &QThread::finished, m_reader, &QObject::deleteLater);
34+
m_reader_thread->start();
35+
QTimer::singleShot(0, m_reader, [] {
36+
util::ThreadRename("qml-debuglog");
37+
});
38+
2539
m_debounce.setSingleShot(true);
2640
m_debounce.setInterval(500);
2741
connect(&m_debounce, &QTimer::timeout, this, [this]() { refresh(); });
2842

2943
connectFileWatcher();
3044
}
3145

46+
DebugLogModel::~DebugLogModel()
47+
{
48+
stop();
49+
if (m_reader_thread) {
50+
m_reader_thread->wait();
51+
}
52+
}
53+
3254
int DebugLogModel::rowCount(const QModelIndex& parent) const
3355
{
3456
if (parent.isValid()) return 0;
@@ -75,9 +97,11 @@ void DebugLogModel::setFilter(const QString& filter)
7597

7698
void DebugLogModel::refresh(bool full_load)
7799
{
100+
if (m_stopping) return;
101+
78102
// Single-read-in-flight guard. If a read is already running, fold this
79103
// request into a trailing re-run rather than piling another job onto the
80-
// thread pool. A burst of watcher events on a noisy node therefore
104+
// worker thread. A burst of watcher events on a noisy node therefore
81105
// collapses to at most two reads: the one in flight, plus one trailer
82106
// that sees the final file state.
83107
if (m_read_in_flight) {
@@ -96,14 +120,32 @@ void DebugLogModel::refresh(bool full_load)
96120
const fs::path path = m_log_path;
97121
const int load_limit = m_load_limit;
98122

99-
auto* watcher = new QFutureWatcher<ReadResult>(this);
100-
connect(watcher, &QFutureWatcher<ReadResult>::finished, this,
101-
[this, watcher, prev_top_content, full_load]() {
102-
onReadCompleted(watcher->result(), prev_top_content, full_load);
103-
watcher->deleteLater();
104-
});
105-
watcher->setFuture(QtConcurrent::run(&DebugLogModel::ReadAndFilter,
106-
path, load_limit, full_load));
123+
if (!m_reader || !m_reader_thread || !m_reader_thread->isRunning()) {
124+
m_read_in_flight = false;
125+
return;
126+
}
127+
128+
const bool queued = QMetaObject::invokeMethod(m_reader,
129+
[this, path, load_limit, full_load, prev_top_content]() mutable {
130+
if (m_read_cancelled.load(std::memory_order_relaxed)) return;
131+
132+
ReadResult result = ReadAndFilter(path, load_limit, full_load, m_read_cancelled);
133+
if (m_read_cancelled.load(std::memory_order_relaxed)) return;
134+
135+
QMetaObject::invokeMethod(this,
136+
[this,
137+
result = std::move(result),
138+
prev_top_content,
139+
full_load]() mutable {
140+
if (m_stopping || m_read_cancelled.load(std::memory_order_relaxed)) return;
141+
onReadCompleted(result, prev_top_content, full_load);
142+
},
143+
Qt::QueuedConnection);
144+
},
145+
Qt::QueuedConnection);
146+
if (!queued) {
147+
m_read_in_flight = false;
148+
}
107149
}
108150

109151
void DebugLogModel::loadMore()
@@ -157,13 +199,41 @@ void DebugLogModel::updateRelativeTimes()
157199
buildDisplayLines();
158200
}
159201

202+
void DebugLogModel::stop()
203+
{
204+
if (m_stopping) return;
205+
206+
m_stopping = true;
207+
m_read_cancelled.store(true, std::memory_order_relaxed);
208+
m_debounce.stop();
209+
210+
const auto watched_files = m_watcher.files();
211+
if (!watched_files.isEmpty()) {
212+
m_watcher.removePaths(watched_files);
213+
}
214+
215+
m_refresh_pending = false;
216+
m_pending_full_load = false;
217+
m_read_in_flight = false;
218+
219+
if (m_reader_thread) {
220+
m_reader_thread->quit();
221+
if (QThread::currentThread() != m_reader_thread) {
222+
m_reader_thread->wait();
223+
}
224+
}
225+
}
226+
160227
// ── Private ──────────────────────────────────────────────────────────────────
161228

162229
DebugLogModel::ReadResult DebugLogModel::ReadAndFilter(const fs::path& log_path,
163230
int load_limit,
164-
bool full_load)
231+
bool full_load,
232+
const std::atomic_bool& cancelled)
165233
{
166234
ReadResult result;
235+
if (cancelled.load(std::memory_order_relaxed)) return result;
236+
167237
const QString path_str = QString::fromStdString(log_path.utf8string());
168238
QFile probe(path_str);
169239
if (!probe.open(QIODevice::ReadOnly | QIODevice::Text)) {
@@ -181,9 +251,14 @@ DebugLogModel::ReadResult DebugLogModel::ReadAndFilter(const fs::path& log_path,
181251
QList<LogLine> filtered;
182252
int fetch_size = load_limit;
183253
while (true) {
184-
const QList<LogLine> raw = ReadRawLines(log_path, fetch_size);
254+
if (cancelled.load(std::memory_order_relaxed)) return {};
255+
256+
const QList<LogLine> raw = ReadRawLines(log_path, fetch_size, cancelled);
257+
if (cancelled.load(std::memory_order_relaxed)) return {};
258+
185259
filtered.clear();
186260
for (const LogLine& l : raw) {
261+
if (cancelled.load(std::memory_order_relaxed)) return {};
187262
if (!l.content.trimmed().isEmpty() || l.timestamp_ms >= 0)
188263
filtered.append(l);
189264
}
@@ -196,8 +271,11 @@ DebugLogModel::ReadResult DebugLogModel::ReadAndFilter(const fs::path& log_path,
196271
}
197272

198273
QList<DebugLogModel::LogLine> DebugLogModel::ReadRawLines(const fs::path& log_path,
199-
int max_lines)
274+
int max_lines,
275+
const std::atomic_bool& cancelled)
200276
{
277+
if (cancelled.load(std::memory_order_relaxed)) return {};
278+
201279
const QString path_str = QString::fromStdString(log_path.utf8string());
202280
QFile file(path_str);
203281
if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) return {};
@@ -213,14 +291,19 @@ QList<DebugLogModel::LogLine> DebugLogModel::ReadRawLines(const fs::path& log_pa
213291

214292
QStringList raw;
215293
raw.reserve(max_lines);
216-
while (!in.atEnd()) raw.append(in.readLine());
294+
while (!in.atEnd()) {
295+
if (cancelled.load(std::memory_order_relaxed)) return {};
296+
raw.append(in.readLine());
297+
}
217298
if (raw.size() > max_lines)
218299
raw = raw.mid(raw.size() - max_lines);
219300

220301
const qint64 now_ms = QDateTime::currentMSecsSinceEpoch();
221302
QList<LogLine> result;
222303
result.reserve(raw.size());
223304
for (const QString& line : raw) {
305+
if (cancelled.load(std::memory_order_relaxed)) return {};
306+
224307
LogLine entry;
225308
const QRegularExpressionMatch m = TIMESTAMP_RX.match(line);
226309
if (m.hasMatch()) {
@@ -243,6 +326,8 @@ void DebugLogModel::onReadCompleted(const ReadResult& result,
243326
const QString& prev_top_content,
244327
bool full_load)
245328
{
329+
if (m_stopping) return;
330+
246331
// Propagate open-error state from the background read.
247332
if (!result.file_opened) {
248333
if (m_open_error != result.error_message) {
@@ -337,7 +422,7 @@ void DebugLogModel::onReadCompleted(const ReadResult& result,
337422

338423
m_read_in_flight = false;
339424
// If changes arrived while we were reading, run one trailing refresh.
340-
if (m_refresh_pending) {
425+
if (!m_stopping && m_refresh_pending) {
341426
m_refresh_pending = false;
342427
const bool do_full = m_pending_full_load;
343428
m_pending_full_load = false;
@@ -352,6 +437,7 @@ void DebugLogModel::connectFileWatcher()
352437
m_watcher.addPath(path_str);
353438
connect(&m_watcher, &QFileSystemWatcher::fileChanged,
354439
this, [this](const QString& path) {
440+
if (m_stopping) return;
355441
m_watcher.addPath(path); // re-add in case of log rotation
356442
m_debounce.start();
357443
});

0 commit comments

Comments
 (0)