From aaedbfae4dbe19911a158ce8d03e897821d37553 Mon Sep 17 00:00:00 2001 From: James Mulcahy Date: Fri, 16 Oct 2026 09:04:59 -0700 Subject: [PATCH] MQTTManager: wait for the web interface's database lock instead of failing The web interface triggers a reload on every save, so a reload's reads often run while that write is still committing. MQTTManager opened SQLite without a busy timeout, so those reads failed at once with "database is locked", the settings getters returned empty strings, and NSPanel's config builder aborted the process on std::stoi(""). Deleting a light and then adding another in quick succession was enough to trigger it. Give every connection a 20 s busy timeout, matching the web interface's. `database` is a static in a header, so each translation unit has its own storage; the timeout is set where each copy is built rather than in init(), which only ever configured main.cpp's copy. Co-Authored-By: Claude Opus 5.5 --- .../database_manager/database_manager.hpp | 21 ++++++++- docker/MQTTManager/tests/database_test.cpp | 44 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 docker/MQTTManager/tests/database_test.cpp diff --git a/docker/MQTTManager/include/database_manager/database_manager.hpp b/docker/MQTTManager/include/database_manager/database_manager.hpp index 41b4a76dc..87a1a4206 100644 --- a/docker/MQTTManager/include/database_manager/database_manager.hpp +++ b/docker/MQTTManager/include/database_manager/database_manager.hpp @@ -103,10 +103,18 @@ struct Entity { void set_entity_data_json(const nlohmann::json &json); }; +// How long a query waits for the web interface to release its lock on the database before +// failing with "database is locked". Matches the "timeout" the web interface uses (settings.py). +static constexpr int DATABASE_BUSY_TIMEOUT_MS = 20000; + +// `database` below is static, so each translation unit that includes this header has its own +// storage, with its own connections. Anything that must apply to every connection is set +// here, where each of those copies is made. +static inline auto make_database() { #if defined(TEST_MODE) && TEST_MODE == 1 -static inline auto database = sqlite_orm::make_storage("/tmp/nspanelmanager_db_test.sqlite3", + auto storage = sqlite_orm::make_storage("/tmp/nspanelmanager_db_test.sqlite3", #else -static inline auto database = sqlite_orm::make_storage("/data/nspanelmanager_db.sqlite3", + auto storage = sqlite_orm::make_storage("/data/nspanelmanager_db.sqlite3", #endif sqlite_orm::make_table("web_roomentitiespage", sqlite_orm::make_column("id", &RoomEntitiesPage::id, sqlite_orm::primary_key().autoincrement()), @@ -175,6 +183,15 @@ static inline auto database = sqlite_orm::make_storage("/data/nspanelmanager_db. sqlite_orm::make_column("entities_page_id", &Entity::entities_page_id), sqlite_orm::make_column("entity_data", &Entity::entity_data), sqlite_orm::make_column("room_id", &Entity::room_id))); + // Without a busy timeout SQLite fails at once while the web interface is writing. A save + // in the web interface is also what triggers a reload, so a reload often reads while the + // write that triggered it is still committing. The failed reads used to return settings + // as empty strings, and std::stoi("") then aborted MQTTManager. + storage.on_open = [](sqlite3 *db) { sqlite3_busy_timeout(db, DATABASE_BUSY_TIMEOUT_MS); }; + return storage; +} + +static inline auto database = make_database(); static void init() { diff --git a/docker/MQTTManager/tests/database_test.cpp b/docker/MQTTManager/tests/database_test.cpp new file mode 100644 index 000000000..9e67a9680 --- /dev/null +++ b/docker/MQTTManager/tests/database_test.cpp @@ -0,0 +1,44 @@ +#include +#include +#include +#include +#include + +namespace { + +// A second connection to the same database file, as the web interface's is. +class OtherConnection { +public: + OtherConnection() { + EXPECT_EQ(sqlite3_open(database_manager::database.filename().c_str(), &_db), SQLITE_OK); + } + ~OtherConnection() { sqlite3_close(_db); } + + void exec(const char *sql) { + char *error = nullptr; + EXPECT_EQ(sqlite3_exec(_db, sql, nullptr, nullptr, &error), SQLITE_OK) << (error ? error : ""); + sqlite3_free(error); + } + +private: + sqlite3 *_db = nullptr; +}; + +} // namespace + +// The web interface triggers a reload on every save, so a reload's reads often run while +// it still holds the write lock. They must wait for it instead of failing with "database +// is locked", which used to make a reload read settings as missing and then crash on +// std::stoi(""). +TEST(DatabaseManagerTest, reads_wait_while_another_connection_holds_the_write_lock) { + OtherConnection web; + web.exec("BEGIN EXCLUSIVE"); + std::thread release([&web] { + std::this_thread::sleep_for(std::chrono::milliseconds(500)); + web.exec("COMMIT"); + }); + + EXPECT_NO_THROW(database_manager::database.count()); + + release.join(); +}