Merge dev into main: Bug Fix - ScheduleManager startup self-deadlock
build / nix-build (push) Successful in 9m50s
build / nix-build (push) Successful in 9m50s
Bug Fix: ScheduleManager::start() held mutex_ and then called loadCache(), which re-locks the same non-recursive mutex. Whenever a schedule cache file was present (the normal state for a provisioned kiosk), this self-deadlocked and blocked SDL_AppInit before the first frame was presented, so the kiosk booted to a black screen showing only the compositor cursor. Narrow the lock scope so loadCache() acquires mutex_ itself, and add a regression test that starts with a cached payload present.
This commit is contained in:
@@ -87,16 +87,20 @@ void ScheduleManager::refreshOnce() {
|
|||||||
|
|
||||||
void ScheduleManager::start(db::DatabaseManager& db, const std::string& timezone,
|
void ScheduleManager::start(db::DatabaseManager& db, const std::string& timezone,
|
||||||
std::function<void()> onChanged) {
|
std::function<void()> onChanged) {
|
||||||
std::lock_guard<std::mutex> lock(mutex_);
|
{
|
||||||
if (started_) return;
|
std::lock_guard<std::mutex> lock(mutex_);
|
||||||
started_ = true;
|
if (started_) return;
|
||||||
db_ = &db;
|
started_ = true;
|
||||||
timezone_ = timezone;
|
db_ = &db;
|
||||||
onChanged_ = std::move(onChanged);
|
timezone_ = timezone;
|
||||||
licenseKey_ = db.getKV(db::KV_LICENSE_KEY);
|
onChanged_ = std::move(onChanged);
|
||||||
enabled_ = true; // config supplies the toggle via setEnabled()
|
licenseKey_ = db.getKV(db::KV_LICENSE_KEY);
|
||||||
|
enabled_ = true; // config supplies the toggle via setEnabled()
|
||||||
|
}
|
||||||
|
|
||||||
// Offline first: load any cached payload immediately.
|
// Offline first: load any cached payload immediately. Called outside the
|
||||||
|
// lock because loadCache() acquires mutex_ itself; holding it here would
|
||||||
|
// self-deadlock on the non-recursive mutex when a cache file exists.
|
||||||
loadCache();
|
loadCache();
|
||||||
|
|
||||||
stop_.store(false);
|
stop_.store(false);
|
||||||
|
|||||||
@@ -297,6 +297,34 @@ TEST(ScheduleManagerTest, OfflineFallsBackToCache) {
|
|||||||
radiantwave::sync::schedule_cache::clear();
|
radiantwave::sync::schedule_cache::clear();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Regression: start() must not deadlock when a cache file is present. It used
|
||||||
|
// to hold mutex_ and then call loadCache(), which re-locked the same
|
||||||
|
// non-recursive mutex (self-deadlock), blocking SDL_AppInit before the first
|
||||||
|
// frame was ever presented — the kiosk sat on a black screen.
|
||||||
|
TEST(ScheduleManagerTest, StartWithCachedPayloadDoesNotDeadlock) {
|
||||||
|
const std::string dir = tempCacheDir();
|
||||||
|
setenv("HOME", dir.c_str(), 1);
|
||||||
|
|
||||||
|
SchedulePayload p = dailyPayload();
|
||||||
|
ASSERT_TRUE(radiantwave::sync::schedule_cache::write(p, "\"cached\""));
|
||||||
|
|
||||||
|
const std::string dbPath =
|
||||||
|
(std::filesystem::temp_directory_path() / "rw_schedule_start_test.db")
|
||||||
|
.string();
|
||||||
|
std::filesystem::remove(dbPath);
|
||||||
|
radiantwave::db::DatabaseManager db;
|
||||||
|
ASSERT_TRUE(db.initialize(dbPath));
|
||||||
|
|
||||||
|
radiantwave::schedule::ScheduleManager manager;
|
||||||
|
manager.start(db, "UTC");
|
||||||
|
EXPECT_TRUE(manager.hasSchedule());
|
||||||
|
manager.shutdown();
|
||||||
|
|
||||||
|
db.close();
|
||||||
|
std::filesystem::remove(dbPath);
|
||||||
|
radiantwave::sync::schedule_cache::clear();
|
||||||
|
}
|
||||||
|
|
||||||
// ─── Phase 5: consume real Portal-generated payloads ───────────────────
|
// ─── Phase 5: consume real Portal-generated payloads ───────────────────
|
||||||
//
|
//
|
||||||
// These fixtures were captured from a running Portal (GET
|
// These fixtures were captured from a running Portal (GET
|
||||||
|
|||||||
Reference in New Issue
Block a user