perf(settings): write settings.json and wallets.json off the UI thread
Settings::save() and WalletIndex::save() ran the atomic write (temp file, fsync, rename, directory fsync) on the UI thread: on every preference change, and for wallets.json after every address-scan pass whose wallet metadata changed, i.e. repeatedly during sync, when the disk is busiest. Each took milliseconds to tens of milliseconds, and seconds under heavy disk load. The JSON is still built on the caller; the write is queued on an App-owned CoalescingWriter, latest wins per file, one write at a time. - After a fund- or security-critical setter (seed-migration stage, encryption pending, PIN enabled, active wallet file, wizard completed) the next save still waits until the file is on disk, so e.g. the sweep-submitted marker is durable before the sweep is broadcast. A failed durable save keeps the next one durable. - Saves before the writer is attached (the upgrade re-save at startup), and in tests and tools, stay synchronous. - main() flushes the queue (at most 3 s) before and after App::shutdown(), before the single-instance lock is released, so a relaunch never reads a stale settings.json. A kill or power loss right after an ordinary save can now lose that last preference change; the previous file stays intact. WalletIndex saves report "queued" rather than "written" (no caller reads it). On a wedged disk, exit can wait up to 2 x 3 s longer, inside the 15 s watchdog. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+7
-2
@@ -363,9 +363,12 @@ bool App::init()
|
||||
// On upgrade (version mismatch), re-save to persist new defaults + current version
|
||||
if (settings_->needsUpgradeSave()) {
|
||||
DEBUG_LOGF("[INFO] Wallet upgraded — re-saving settings with new defaults\n");
|
||||
settings_->save();
|
||||
settings_->save(); // no writer attached yet: synchronous
|
||||
settings_->clearUpgradeSave();
|
||||
}
|
||||
// From here on settings_->save() is written by file_writer_ off the UI thread (flushed by main()
|
||||
// at exit); a save after a fund/security-critical setter still waits for the disk (Settings::save).
|
||||
settings_->setFileWriter(&file_writer_);
|
||||
|
||||
// Lite builds with a linked SDXL backend own a lite wallet controller that drives
|
||||
// real create/open/restore through the bridge. Full-node and unlinked-lite builds
|
||||
@@ -575,7 +578,9 @@ bool App::init()
|
||||
address_book_.load();
|
||||
|
||||
// Load the wallet-files metadata index (wallets.json); populated after each wallet load. Local
|
||||
// only, missing file is fine.
|
||||
// only, missing file is fine. Its saves are queued on file_writer_ (they follow every scan pass
|
||||
// whose metadata changed, so they shouldn't fsync on the UI thread).
|
||||
wallet_index_.setFileWriter(&file_writer_);
|
||||
wallet_index_.load();
|
||||
|
||||
// If this wallet build bundles a newer daemon than the one already installed, offer to update it
|
||||
|
||||
@@ -36,6 +36,7 @@
|
||||
#include "services/wallet_security_controller.h"
|
||||
#include "services/wallet_security_workflow.h"
|
||||
#include "util/async_task_manager.h"
|
||||
#include "util/coalescing_writer.h"
|
||||
#include "util/fetch_backoff.h"
|
||||
#include "util/pool_stats_service.h"
|
||||
#include "wallet/wallet_capabilities.h"
|
||||
@@ -161,6 +162,12 @@ public:
|
||||
*/
|
||||
void shutdown();
|
||||
|
||||
/**
|
||||
* @brief Wait (bounded, 3 s) for queued settings.json / wallets.json writes to land on disk.
|
||||
* main() calls it around shutdown(): _Exit() skips ~App(), so nothing else would.
|
||||
*/
|
||||
void flushPendingWrites() { file_writer_.flush(std::chrono::milliseconds(3000)); }
|
||||
|
||||
/**
|
||||
* @brief Check if app should exit (render loop can stop)
|
||||
* Enforces minimum 1-second display of shutdown screen so user sees feedback.
|
||||
@@ -871,6 +878,7 @@ private:
|
||||
// Saved connection credentials (needed to open the fast-lane connection)
|
||||
rpc::ConnectionConfig saved_config_;
|
||||
|
||||
util::CoalescingWriter file_writer_; // background atomic writer for settings.json / wallets.json
|
||||
std::unique_ptr<config::Settings> settings_;
|
||||
std::unique_ptr<wallet::LiteWalletController> lite_wallet_; // lite builds w/ linked backend
|
||||
// Pending send_tab callback for an in-flight lite send (delivered in update() once the
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
#include <algorithm>
|
||||
#include <type_traits>
|
||||
|
||||
#include "../util/coalescing_writer.h"
|
||||
#include "../util/logger.h"
|
||||
#include "../util/platform.h"
|
||||
|
||||
@@ -657,8 +658,13 @@ bool Settings::save(const std::string& path)
|
||||
// Atomic + durable: write to a temp file, fsync, then rename over the real file.
|
||||
// A crash mid-write can no longer truncate settings.json (which would silently
|
||||
// reset every preference on the next launch). Owner-only (0600) — it carries the
|
||||
// lite-server list and address metadata.
|
||||
return util::Platform::writeFileAtomically(path, j.dump(4), /*restrictPermissions=*/true);
|
||||
// lite-server list and address metadata. With a writer attached the same atomic write
|
||||
// runs on its background thread (queued, latest-wins); after a critical setter
|
||||
// (durable_next_save_) this waits until the file is on disk, as the old save did.
|
||||
const bool ok = util::persistFileAtomically(writer_, durable_next_save_, path, j.dump(4),
|
||||
/*restrictPermissions=*/true);
|
||||
if (ok) durable_next_save_ = false;
|
||||
return ok;
|
||||
} catch (const std::exception& e) {
|
||||
DEBUG_LOGF("Failed to save settings: %s\n", e.what());
|
||||
return false;
|
||||
|
||||
+24
-11
@@ -12,6 +12,7 @@
|
||||
#include <vector>
|
||||
|
||||
namespace dragonx {
|
||||
namespace util { class CoalescingWriter; }
|
||||
namespace config {
|
||||
|
||||
/**
|
||||
@@ -39,10 +40,16 @@ public:
|
||||
|
||||
/**
|
||||
* @brief Save settings to default location
|
||||
* @return true if saved successfully
|
||||
* With a file writer attached (the App's), the JSON snapshot is taken here, on the caller's thread,
|
||||
* and the atomic write runs on the writer's thread: returns true once queued. After a fund/security-
|
||||
* critical setter (see durable_next_save_) it waits until the file is on disk, like the old save().
|
||||
* @return true if saved (or queued) successfully
|
||||
*/
|
||||
bool save();
|
||||
|
||||
/** @brief Route saves through a background writer (not owned; must outlive this). Null = synchronous. */
|
||||
void setFileWriter(util::CoalescingWriter* writer) { writer_ = writer; }
|
||||
|
||||
/**
|
||||
* @brief Save settings to specific path
|
||||
* @param path Path to settings file
|
||||
@@ -344,7 +351,7 @@ public:
|
||||
|
||||
// First-run wizard
|
||||
bool getWizardCompleted() const { return wizard_completed_; }
|
||||
void setWizardCompleted(bool v) { wizard_completed_ = v; }
|
||||
void setWizardCompleted(bool v) { wizard_completed_ = v; durable_next_save_ = true; }
|
||||
|
||||
// Whether the one-time "back up your seed phrase" reminder has already been shown.
|
||||
bool getSeedBackupReminded() const { return seed_backup_reminded_; }
|
||||
@@ -366,7 +373,7 @@ public:
|
||||
// observed to be actually encrypted. Lets a quit/crash/failed-connect before it applies be detected
|
||||
// and surfaced (W2-2). NEVER stores the passphrase — only the fact that encryption was requested.
|
||||
bool getEncryptionPending() const { return encryption_pending_; }
|
||||
void setEncryptionPending(bool v) { encryption_pending_ = v; }
|
||||
void setEncryptionPending(bool v) { encryption_pending_ = v; durable_next_save_ = true; }
|
||||
|
||||
// Bundled-daemon size we last prompted to install (see App::renderDaemonUpdatePrompt). Lets the
|
||||
// "a newer node is bundled — update?" prompt fire once per wallet version, never re-nagging.
|
||||
@@ -377,31 +384,31 @@ public:
|
||||
// the datadir; defaults to the daemon's own default. Used at launch to know which wallet we're
|
||||
// on before connect + to scope per-wallet data.
|
||||
std::string getActiveWalletFile() const { return active_wallet_file_; }
|
||||
void setActiveWalletFile(const std::string& v) { active_wallet_file_ = v; }
|
||||
void setActiveWalletFile(const std::string& v) { active_wallet_file_ = v; durable_next_save_ = true; }
|
||||
|
||||
// Pending "migrate to a seed wallet" state (Phase 1 created the wallet; a later sweep/adopt
|
||||
// step consumes it). dest = the new wallet's sweep-target z-address; tempDir = its datadir.
|
||||
bool getSeedMigrationPending() const { return seed_migration_pending_; }
|
||||
void setSeedMigrationPending(bool v) { seed_migration_pending_ = v; }
|
||||
void setSeedMigrationPending(bool v) { seed_migration_pending_ = v; durable_next_save_ = true; }
|
||||
std::string getSeedMigrationDest() const { return seed_migration_dest_; }
|
||||
void setSeedMigrationDest(const std::string& v) { seed_migration_dest_ = v; }
|
||||
void setSeedMigrationDest(const std::string& v) { seed_migration_dest_ = v; durable_next_save_ = true; }
|
||||
std::string getSeedMigrationTempDir() const { return seed_migration_temp_dir_; }
|
||||
void setSeedMigrationTempDir(const std::string& v) { seed_migration_temp_dir_ = v; }
|
||||
void setSeedMigrationTempDir(const std::string& v) { seed_migration_temp_dir_ = v; durable_next_save_ = true; }
|
||||
// The sweep transaction id, persisted once the sweep is submitted — non-empty means the
|
||||
// migration is past the sweep, so a resume goes to the confirm/adopt stage (not sweep again).
|
||||
std::string getSeedMigrationSweepTxid() const { return seed_migration_sweep_txid_; }
|
||||
void setSeedMigrationSweepTxid(const std::string& v) { seed_migration_sweep_txid_ = v; }
|
||||
void setSeedMigrationSweepTxid(const std::string& v) { seed_migration_sweep_txid_ = v; durable_next_save_ = true; }
|
||||
// W3-3: the async sweep operation id, persisted while the sweep is in flight (before it resolves
|
||||
// to a txid). Lets a resume re-poll a mid-sweep interruption instead of dropping the txid. Cleared
|
||||
// in the same write that persists the txid, so the txid always outranks it (see [[decideSeedMigrationResume]]).
|
||||
std::string getSeedMigrationSweepOpid() const { return seed_migration_sweep_opid_; }
|
||||
void setSeedMigrationSweepOpid(const std::string& v) { seed_migration_sweep_opid_ = v; }
|
||||
void setSeedMigrationSweepOpid(const std::string& v) { seed_migration_sweep_opid_ = v; durable_next_save_ = true; }
|
||||
// Set TRUE synchronously when a sweep is broadcast; survives the stale-opid resume fallback so the
|
||||
// Sweep step knows a ~0 legacy balance may be an UNCONFIRMED in-flight sweep (funds not yet safe)
|
||||
// and must NOT short-circuit to adopt/swap wallet.dat. Cleared on confirmed-adopt / reset / a
|
||||
// failed submit. See [[decideSeedMigrationResume]] and App::renderSeedMigrationDialog (fund-safety).
|
||||
bool getSeedMigrationSweepSubmitted() const { return seed_migration_sweep_submitted_; }
|
||||
void setSeedMigrationSweepSubmitted(bool v) { seed_migration_sweep_submitted_ = v; }
|
||||
void setSeedMigrationSweepSubmitted(bool v) { seed_migration_sweep_submitted_ = v; durable_next_save_ = true; }
|
||||
|
||||
// Security — auto-lock timeout (seconds; 0 = disabled)
|
||||
int getAutoLockTimeout() const { return auto_lock_timeout_; }
|
||||
@@ -413,7 +420,7 @@ public:
|
||||
|
||||
// Security — PIN unlock enabled
|
||||
bool getPinEnabled() const { return pin_enabled_; }
|
||||
void setPinEnabled(bool v) { pin_enabled_ = v; }
|
||||
void setPinEnabled(bool v) { pin_enabled_ = v; durable_next_save_ = true; }
|
||||
|
||||
// Daemon — keep running in background when closing the app
|
||||
bool getKeepDaemonRunning() const { return keep_daemon_running_; }
|
||||
@@ -581,6 +588,12 @@ public:
|
||||
|
||||
private:
|
||||
std::string settings_path_;
|
||||
util::CoalescingWriter* writer_ = nullptr; // null -> synchronous saves (tests, tools, early startup)
|
||||
// Armed by the fund/security-critical setters (seed-migration stage, encryption_pending, pin_enabled,
|
||||
// active_wallet_file, wizard_completed): the next save() waits until the file is on disk before it
|
||||
// returns, so e.g. the sweep-submitted marker is durable before the sweep is broadcast. Cleared by a
|
||||
// successful save. A NEW setter for crash-critical state must arm it too.
|
||||
bool durable_next_save_ = false;
|
||||
|
||||
// Settings values
|
||||
std::string theme_ = "dragonx";
|
||||
|
||||
@@ -9,6 +9,7 @@
|
||||
#include <filesystem>
|
||||
#include <fstream>
|
||||
|
||||
#include "../util/coalescing_writer.h"
|
||||
#include "../util/logger.h"
|
||||
#include "../util/platform.h"
|
||||
|
||||
@@ -109,7 +110,8 @@ bool WalletIndex::save()
|
||||
j["extraFolders"] = extra_folders_;
|
||||
|
||||
// No secrets here (file names + coarse metadata) but keep it owner-only for consistency.
|
||||
if (!util::Platform::writeFileAtomically(file_path_, j.dump(2), /*restrictPermissions=*/true)) {
|
||||
if (!util::persistFileAtomically(writer_, /*waitForDisk=*/false, file_path_, j.dump(2),
|
||||
/*restrictPermissions=*/true)) {
|
||||
DEBUG_LOGF("Could not write wallet index: %s\n", file_path_.c_str());
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
#include <vector>
|
||||
|
||||
namespace dragonx {
|
||||
namespace util { class CoalescingWriter; }
|
||||
namespace data {
|
||||
|
||||
/**
|
||||
@@ -41,9 +42,12 @@ struct WalletIndexEntry {
|
||||
class WalletIndex {
|
||||
public:
|
||||
bool load();
|
||||
bool save();
|
||||
bool save(); // queued on the writer when one is attached (returns true once queued)
|
||||
static std::string getDefaultPath();
|
||||
|
||||
/** @brief Route save() through a background writer (not owned; must outlive this). Null = synchronous. */
|
||||
void setFileWriter(util::CoalescingWriter* writer) { writer_ = writer; }
|
||||
|
||||
const std::vector<WalletIndexEntry>& entries() const { return entries_; }
|
||||
const std::vector<std::string>& extraFolders() const { return extra_folders_; }
|
||||
|
||||
@@ -64,6 +68,7 @@ private:
|
||||
std::vector<WalletIndexEntry> entries_;
|
||||
std::vector<std::string> extra_folders_;
|
||||
std::string file_path_;
|
||||
util::CoalescingWriter* writer_ = nullptr; // null -> synchronous writes
|
||||
};
|
||||
|
||||
} // namespace data
|
||||
|
||||
@@ -2357,8 +2357,14 @@ int main(int argc, char* argv[])
|
||||
_Exit(0);
|
||||
}).detach();
|
||||
|
||||
// Land the queued settings/wallets.json writes (incl. the window size saved above) before the joins
|
||||
// in shutdown(); bounded (3 s) so a wedged disk can't eat the watchdog's budget.
|
||||
app.flushPendingWrites();
|
||||
// Final cleanup (daemon already stopped by beginShutdown)
|
||||
app.shutdown();
|
||||
// shutdown()'s fallback branch saves settings again; land that too, BEFORE the single-instance lock
|
||||
// is released below so a relaunch can't read a stale settings.json. _Exit() skips ~App().
|
||||
app.flushPendingWrites();
|
||||
#ifdef DRAGONX_USE_DX11
|
||||
Shutdown(window, dx);
|
||||
#else
|
||||
|
||||
@@ -8138,6 +8138,69 @@ void testAtomicFileWrite()
|
||||
removeTree(dir);
|
||||
}
|
||||
|
||||
#ifndef _WIN32
|
||||
// C269: Settings + WalletIndex saves through the App's background writer, under a temp $HOME (as
|
||||
// testWalletIndex does; Windows' config dir can't be redirected). The HOME redirect is what keeps a
|
||||
// developer's real settings.json untouched (getConfigDir() uses only $HOME/.config); XDG_CONFIG_HOME is
|
||||
// redirected too purely as a precaution. A queued save lands after flush(); a critical setter
|
||||
// makes the next save wait for the disk (readable with NO flush), and an older queued snapshot can't land
|
||||
// after it.
|
||||
void testSettingsBackgroundSave()
|
||||
{
|
||||
using ms = std::chrono::milliseconds;
|
||||
const char* oldHome = std::getenv("HOME");
|
||||
const std::string savedHome = oldHome ? oldHome : "";
|
||||
const char* oldXdg = std::getenv("XDG_CONFIG_HOME");
|
||||
const bool hadXdg = oldXdg != nullptr;
|
||||
const std::string savedXdg = hadXdg ? oldXdg : "";
|
||||
const fs::path tmp = makeTempDir();
|
||||
setenv("HOME", tmp.string().c_str(), 1);
|
||||
setenv("XDG_CONFIG_HOME", (tmp / ".config").string().c_str(), 1);
|
||||
{
|
||||
dragonx::util::CoalescingWriter w; // declared first: outlives s and idx
|
||||
dragonx::config::Settings s;
|
||||
s.setFileWriter(&w);
|
||||
s.setUIOpacity(0.55f);
|
||||
EXPECT_TRUE(s.save()); // queued
|
||||
s.setUIOpacity(0.65f);
|
||||
EXPECT_TRUE(s.save()); // coalesces over / follows the first
|
||||
EXPECT_TRUE(w.flush(ms(5000)));
|
||||
{
|
||||
dragonx::config::Settings back;
|
||||
EXPECT_TRUE(back.load());
|
||||
EXPECT_NEAR(back.getUIOpacity(), 0.65, 1e-6);
|
||||
}
|
||||
// A critical setter makes the next save wait for the disk: readable with NO flush, and the
|
||||
// queued older snapshot can't land after it.
|
||||
s.setUIOpacity(0.70f);
|
||||
EXPECT_TRUE(s.save()); // queued
|
||||
s.setPinEnabled(true);
|
||||
EXPECT_TRUE(s.save()); // durable
|
||||
{
|
||||
dragonx::config::Settings back;
|
||||
EXPECT_TRUE(back.load());
|
||||
EXPECT_TRUE(back.getPinEnabled());
|
||||
EXPECT_NEAR(back.getUIOpacity(), 0.70, 1e-6);
|
||||
}
|
||||
EXPECT_EQ(w.pendingCount(), (size_t)0);
|
||||
|
||||
dragonx::data::WalletIndex idx;
|
||||
idx.setFileWriter(&w);
|
||||
idx.load();
|
||||
dragonx::data::WalletIndexEntry e; e.fileName = "wallet.dat"; e.displayName = "Main";
|
||||
idx.upsert(e);
|
||||
EXPECT_TRUE(idx.save()); // queued
|
||||
EXPECT_TRUE(w.flush(ms(5000)));
|
||||
dragonx::data::WalletIndex idx2;
|
||||
EXPECT_TRUE(idx2.load());
|
||||
EXPECT_TRUE(idx2.find("wallet.dat") != nullptr);
|
||||
}
|
||||
if (!savedHome.empty()) setenv("HOME", savedHome.c_str(), 1); else unsetenv("HOME");
|
||||
if (hadXdg) setenv("XDG_CONFIG_HOME", savedXdg.c_str(), 1); else unsetenv("XDG_CONFIG_HOME");
|
||||
removeTree(tmp);
|
||||
}
|
||||
#endif
|
||||
|
||||
#ifndef _WIN32
|
||||
// C218: openUrl/openFolder must return without waiting for the opener. Fake xdg-open/open that sleep 3 s
|
||||
// are the ONLY entries on PATH (posix_spawnp keeps searching after EACCES, so on a noexec temp dir an
|
||||
@@ -12665,6 +12728,7 @@ int main()
|
||||
testCoalescingWriterOrdering();
|
||||
testAtomicFileWrite();
|
||||
#ifndef _WIN32
|
||||
testSettingsBackgroundSave();
|
||||
testOpenerDoesNotBlock();
|
||||
#endif
|
||||
testHushChatCrypto();
|
||||
|
||||
Reference in New Issue
Block a user