mirror of
https://github.com/agessaman/MeshCore.git
synced 2026-08-27 22:34:14 +00:00
fix(mqtt): renew JWT credentials without stopping the esp-mqtt client
The scheduled JWT bounce called PsychicMqttClient::disconnect(), which ends
with esp_mqtt_client_stop(). That ends the client task and returns its 6 KiB
stack to the heap at the moment the TLS teardown vacates two 16 KiB mbedTLS
record buffers, so the stack lands in that hole and the next handshake cannot
reuse it. On non-PSRAM boards the largest free block then ratchets down 16 KiB
at a time while total free heap stays flat.
Soak evidence from a Heltec V3 on 8d1a0eb3: 43 of 60 disconnects had no
preceding transport error, i.e. they were this proactive bounce rather than a
broker FIN, and two of the three max_alloc steps landed within 5 s of one.
Losing a whole TLS session later returned exactly 16,384 bytes of contiguity.
softDisconnect() closes the transport without the stop, so the task and its
stack stay put across the handshake. The bounce uses it plus reconnect(), and
falls back to connect() when the client really is stopped, since reconnect()
is a silent no-op in that state.
Also corrects a comment claiming the mbedTLS context survives a transport
close: only the esp-mqtt client object does.
(cherry picked from commit 10cf5cf48fb009e751e25b37fcc1f3d1256ddbbc)
This commit is contained in:
@@ -433,7 +433,12 @@ void PsychicMqttClient::connect()
|
||||
}
|
||||
}
|
||||
|
||||
ESP_ERROR_CHECK_WITHOUT_ABORT(esp_mqtt_client_start(_client));
|
||||
esp_err_t start_result = esp_mqtt_client_start(_client);
|
||||
ESP_ERROR_CHECK_WITHOUT_ABORT(start_result);
|
||||
if (start_result == ESP_OK)
|
||||
{
|
||||
_started = true;
|
||||
}
|
||||
ESP_LOGI(TAG, "MQTT client started.");
|
||||
}
|
||||
|
||||
@@ -489,9 +494,40 @@ void PsychicMqttClient::disconnect()
|
||||
}
|
||||
|
||||
esp_mqtt_client_stop(_client);
|
||||
_started = false;
|
||||
ESP_LOGI(TAG, "MQTT client stopped.");
|
||||
}
|
||||
|
||||
void PsychicMqttClient::softDisconnect(unsigned long timeout_ms)
|
||||
{
|
||||
if (_client == nullptr)
|
||||
{
|
||||
ESP_LOGW(TAG, "MQTT client not started.");
|
||||
return;
|
||||
}
|
||||
|
||||
if (!_connected)
|
||||
{
|
||||
// Nothing to close; leaving the task alone is the whole point.
|
||||
return;
|
||||
}
|
||||
|
||||
ESP_LOGI(TAG, "Disconnecting MQTT transport (client task retained).");
|
||||
_stopMqttClient = false;
|
||||
esp_mqtt_client_disconnect(_client);
|
||||
|
||||
unsigned long waited = 0;
|
||||
while (!_stopMqttClient && waited < timeout_ms)
|
||||
{
|
||||
vTaskDelay(10 / portTICK_PERIOD_MS);
|
||||
waited += 10;
|
||||
}
|
||||
if (!_stopMqttClient)
|
||||
{
|
||||
ESP_LOGW(TAG, "softDisconnect: no DISCONNECTED event in %lums", timeout_ms);
|
||||
}
|
||||
}
|
||||
|
||||
void PsychicMqttClient::forceStop()
|
||||
{
|
||||
if (_client == nullptr)
|
||||
@@ -506,6 +542,7 @@ void PsychicMqttClient::forceStop()
|
||||
}
|
||||
ESP_ERROR_CHECK_WITHOUT_ABORT(esp_mqtt_client_stop(_client));
|
||||
_connected = false;
|
||||
_started = false;
|
||||
ESP_LOGI(TAG, "MQTT client forcefully stopped.");
|
||||
}
|
||||
|
||||
|
||||
@@ -368,6 +368,29 @@ public:
|
||||
*/
|
||||
void disconnect();
|
||||
|
||||
/**
|
||||
* @brief Closes the transport but leaves the client task running.
|
||||
*
|
||||
* disconnect() ends with esp_mqtt_client_stop(), which ends the client task
|
||||
* and returns its 6 KiB stack to the heap right as a TLS handshake vacates
|
||||
* two 16 KiB mbedTLS record buffers — the stack then lands in that hole and
|
||||
* the largest free block ratchets down. This variant omits the stop, so the
|
||||
* task and its stack stay put. Pair it with reconnect().
|
||||
*
|
||||
* @param timeout_ms how long to wait for the DISCONNECTED event before
|
||||
* giving up. Bounded on purpose: disconnect()'s wait is
|
||||
* unbounded and a lost event would wedge the caller.
|
||||
*/
|
||||
void softDisconnect(unsigned long timeout_ms = 5000);
|
||||
|
||||
/**
|
||||
* @brief True once esp_mqtt_client_start() has succeeded and no stop has run.
|
||||
*
|
||||
* reconnect() silently does nothing on a stopped client, so callers that
|
||||
* want to avoid stop/start must check this and fall back to connect().
|
||||
*/
|
||||
bool isStarted() const { return _started; }
|
||||
|
||||
/**
|
||||
* @brief Forcefully stops the MQTT client and disconnects from the server.
|
||||
* This does not trigger the onDisconnect callbacks.
|
||||
@@ -478,6 +501,7 @@ private:
|
||||
bool _connected = false;
|
||||
bool _stopMqttClient = false;
|
||||
bool _config_dirty = true;
|
||||
bool _started = false;
|
||||
|
||||
// Runtime cap on the esp-mqtt outbox for QoS 0 async publishes (bytes).
|
||||
// 0 = disabled. Enforced in publish(); not an esp-mqtt config field.
|
||||
|
||||
@@ -1775,9 +1775,11 @@ bool MQTTBridge::setupSlot(int index) {
|
||||
}
|
||||
|
||||
// Reconfigure path: if we're re-applying (e.g. after a preset change), stop
|
||||
// the existing connection cleanly first. The client object (and its mbedTLS
|
||||
// context) is reused; setCredentials / setServer below overwrite the config
|
||||
// fields in place before connect() restarts the ESP-IDF client.
|
||||
// the existing connection cleanly first. The client object is reused, but its
|
||||
// mbedTLS context is NOT — closing the transport destroys the TLS session,
|
||||
// record buffers, and peer certificate, and the next connect() reallocates
|
||||
// them. setCredentials / setServer below overwrite the config fields in place
|
||||
// before connect() restarts the ESP-IDF client.
|
||||
if (slot.initial_connect_done) {
|
||||
if (slot.client->connected()) {
|
||||
slot.client->disconnect();
|
||||
@@ -2140,11 +2142,24 @@ void MQTTBridge::maintainSlotConnection(int index, unsigned long now_millis, uns
|
||||
// Disconnect + reconnect with fresh credentials, reusing existing client
|
||||
// to avoid internal heap leak/fragmentation from destroy/create cycles
|
||||
MQTT_DEBUG_PRINTLN("MQTT%d token renewal: reconnecting with fresh credentials", index + 1);
|
||||
if (slot.client->connected()) {
|
||||
slot.client->disconnect(); // stops the client internally
|
||||
MQTT_TRACE_HEAP("renewal:before-bounce", index);
|
||||
if (slot.client->isStarted()) {
|
||||
// Keep the esp-mqtt task alive across the handshake. disconnect()
|
||||
// would stop it, returning its 6 KiB stack into the hole the two
|
||||
// 16 KiB mbedTLS record buffers just vacated — which is what walks
|
||||
// the largest free block down 16 KiB at a time on non-PSRAM boards.
|
||||
slot.client->softDisconnect();
|
||||
MQTT_TRACE_HEAP("renewal:after-disconnect", index);
|
||||
slot.client->setCredentials(_jwt_username, slot.auth_token);
|
||||
MQTT_TRACE_HEAP("renewal:after-credentials", index);
|
||||
slot.client->reconnect();
|
||||
} else {
|
||||
// Client was stopped (teardown/reconfigure). reconnect() is a no-op
|
||||
// on a stopped client, so this path must start it.
|
||||
slot.client->setCredentials(_jwt_username, slot.auth_token);
|
||||
slot.client->connect();
|
||||
}
|
||||
slot.client->setCredentials(_jwt_username, slot.auth_token);
|
||||
slot.client->connect(); // restart stopped client; reconnect() fails silently on a stopped client
|
||||
MQTT_TRACE_HEAP("renewal:after-reconnect", index);
|
||||
reconnect_attempted = true;
|
||||
_last_slot_reconnect_ms = now_millis;
|
||||
MQTT_DEBUG_PRINTLN("MQTT%d int_heap=%d at token renewal reconnect", index + 1,
|
||||
|
||||
@@ -36,6 +36,19 @@ class MeshSNMPAgent; // Forward declaration
|
||||
#define MQTT_DEBUG_PRINTLN(...) {}
|
||||
#endif
|
||||
|
||||
// Largest-free-block trace around the reconnect lifecycle. On non-PSRAM boards the
|
||||
// mbedTLS record buffers are two 16 KiB internal-DRAM blocks, so what matters is the
|
||||
// largest contiguous block, not the free total — a soak can show flat free heap while
|
||||
// max_alloc walks down. Costs two heap_caps calls per reconnect, so it stays on.
|
||||
#if defined(MQTT_DEBUG) && defined(ARDUINO) && defined(ESP32)
|
||||
#define MQTT_TRACE_HEAP(point, idx) \
|
||||
MQTT_DEBUG_PRINTLN("HEAPTRACE slot=%d %s free=%u max=%u", (int)(idx) + 1, point, \
|
||||
(unsigned)heap_caps_get_free_size(MALLOC_CAP_INTERNAL), \
|
||||
(unsigned)heap_caps_get_largest_free_block(MALLOC_CAP_INTERNAL))
|
||||
#else
|
||||
#define MQTT_TRACE_HEAP(point, idx) do {} while(0)
|
||||
#endif
|
||||
|
||||
#ifdef WITH_MQTT_BRIDGE
|
||||
|
||||
// Periodic neighbors publication keys off the mesh neighbor cache (sized by
|
||||
|
||||
Reference in New Issue
Block a user