From 515eb2a16bdc1367969ca6d8c079e3a55bf4ab62 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 13 Jun 2026 06:59:43 +0000 Subject: [PATCH 1/2] fix: run message/sendmail shunting-yard synchronously to avoid main-loop deadlock command_message() and command_sendmail() handed their value to WebSchedulerService via raw_value and busy-waited up to 2s for the scheduler loop (running in a separate task) to compute it. After the scheduler was moved to run synchronously in the main loop, any caller running in the main loop (MQTT-triggered commands, scheduler-triggered commands) deadlocks: the loop that would compute raw_value cannot run while the caller is blocking inside it. The 2s wait then times out and system/message fails entirely (sendmail sends the un-computed body). Compute the value directly with compute() instead, which restores correct behaviour for all callers. Co-authored-by: Proddy --- src/core/system.cpp | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/src/core/system.cpp b/src/core/system.cpp index 208e657c5..0bf2ec654 100644 --- a/src/core/system.cpp +++ b/src/core/system.cpp @@ -32,6 +32,7 @@ #include #include "firmwareVersion.h" +#include "shuntingYard.h" // for compute() used by the message and sendmail commands #if defined(EMSESP_TEST) #include "../test/test.h" @@ -194,15 +195,11 @@ bool System::command_sendmail(const char * value, const int8_t id) { // msg.headers.addCustom("Importance", PRIORITY); // msg.headers.addCustom("X-MSMail-Priority", PRIORITY); // msg.headers.addCustom("X-Priority", PRIORITY_NUM); - EMSESP::webSchedulerService.computed_value.clear(); - EMSESP::webSchedulerService.raw_value = body.c_str(); - for (uint16_t wait = 0; wait < 2000 && !EMSESP::webSchedulerService.raw_value.empty(); wait++) { - delay(1); - } - if (!EMSESP::webSchedulerService.computed_value.empty()) { - body = EMSESP::webSchedulerService.computed_value.c_str(); - EMSESP::webSchedulerService.computed_value.clear(); - EMSESP::webSchedulerService.computed_value.shrink_to_fit(); // free allocated memory + // run the body through the Shunting Yard calculator (entity substitution, expressions, optional {url} fetch) + // keep the original body if the calculator returns nothing + std::string computed_body = compute(body.c_str()); + if (!computed_body.empty()) { + body = computed_body.c_str(); } msg.text.body(body); @@ -344,22 +341,16 @@ bool System::command_message(const char * value, const int8_t id, JsonObject out return false; // must have a string value } - EMSESP::webSchedulerService.computed_value.clear(); - EMSESP::webSchedulerService.raw_value = value; - for (uint16_t wait = 0; wait < 2000 && !EMSESP::webSchedulerService.raw_value.empty(); wait++) { - delay(1); - } - - if (EMSESP::webSchedulerService.computed_value.empty()) { + // process the message via the Shunting Yard calculator (entity substitution, expressions, optional {url} fetch) + std::string computed_value = compute(value); + if (computed_value.empty()) { LOG_WARNING("Message result is empty"); return false; } - LOG_INFO("Message: %s", EMSESP::webSchedulerService.computed_value.c_str()); // send to log - Mqtt::queue_publish(F_(message), EMSESP::webSchedulerService.computed_value); // send to MQTT if enabled - output["api_data"] = EMSESP::webSchedulerService.computed_value; // send to API - EMSESP::webSchedulerService.computed_value.clear(); - EMSESP::webSchedulerService.computed_value.shrink_to_fit(); + LOG_INFO("Message: %s", computed_value.c_str()); // send to log + Mqtt::queue_publish(F_(message), computed_value); // send to MQTT if enabled + output["api_data"] = computed_value; // send to API return true; } From 5ee08443f9b7cc283e95ed6f6164342c0051cba1 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 13 Jun 2026 07:02:28 +0000 Subject: [PATCH 2/2] refactor: drop dead scheduler raw_value deferral, route url messages to worker The raw_value/computed_value handoff on WebSchedulerService is no longer used now that command_message()/command_sendmail() compute synchronously, so remove the members and the dead block in the scheduler loop. Also drop the system/message special-case in dispatchCommand(): a system/message whose value embeds a {url} now blocks at execution time like any other internal command, so it should be offloaded to the command worker task (large stack, off the main loop) instead of running inline. Update the now-stale comments. Co-authored-by: Proddy --- src/web/WebCommandService.cpp | 18 ++++++++---------- src/web/WebSchedulerService.cpp | 5 ----- src/web/WebSchedulerService.h | 3 --- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/web/WebCommandService.cpp b/src/web/WebCommandService.cpp index 8da74bb35..6f073d2cf 100644 --- a/src/web/WebCommandService.cpp +++ b/src/web/WebCommandService.cpp @@ -141,14 +141,12 @@ bool WebCommandService::dispatchCommand(const char * name, const char * value) { if (isUrlCommand(ci->cmd.c_str())) { return queueCommand(name, value); } - // system/message defers evaluation of its value (via the scheduler's raw_value), - // so executing it never blocks - keep it synchronous even if the value has a {url} - if (Helpers::toLower(ci->cmd.c_str()) != "system/message") { - // the effective value is the override if given, else the command's stored default - const std::string effective_value = value ? value : std::string(ci->value.c_str()); - if (valueContainsUrl(effective_value)) { - return queueCommand(name, value); - } + // internal command whose value embeds a {url} fetch (e.g. system/message) - the value is + // resolved by compute() at execution time and would block, so offload it to the worker task + // the effective value is the override if given, else the command's stored default + const std::string effective_value = value ? value : std::string(ci->value.c_str()); + if (valueContainsUrl(effective_value)) { + return queueCommand(name, value); } } } @@ -240,8 +238,8 @@ bool WebCommandService::executeCommand(const char * name, const std::string & co // run the value through the shunting-yard calculator so expressions like "custom/heatcnt + 1" // are resolved (entity references replaced by their values, then computed). Plain values pass // through unchanged. Applies to both URL and internal commands, like the old scheduler code - // which computed the value before executing. system/message evaluates its own argument later - // (deferred via the scheduler's raw_value), so pre-computing it would run it twice - pass raw. + // which computed the value before executing. system/message runs the shunting-yard on its own + // argument, so pre-computing it here would run it twice - pass it through raw. std::string computed_data = data; if (!data.empty() && cmd != "system/message") { computed_data = compute(data); diff --git a/src/web/WebSchedulerService.cpp b/src/web/WebSchedulerService.cpp index 79c3fe097..d788e0768 100644 --- a/src/web/WebSchedulerService.cpp +++ b/src/web/WebSchedulerService.cpp @@ -367,11 +367,6 @@ void WebSchedulerService::loop() { static uint32_t last_uptime_min = 0; static uint32_t last_uptime_sec = 0; - if (!raw_value.empty()) { - computed_value = compute(raw_value); - raw_value.clear(); - } - if (scheduleItems_->empty()) { return; } diff --git a/src/web/WebSchedulerService.h b/src/web/WebSchedulerService.h index db21f38cf..299ddd2fa 100644 --- a/src/web/WebSchedulerService.h +++ b/src/web/WebSchedulerService.h @@ -75,9 +75,6 @@ class WebSchedulerService : public StatefulService { std::string get_metrics_prometheus(); - std::string raw_value; - std::string computed_value; - #if defined(EMSESP_TEST) void load_test_data(); #endif