From de4b352495fb88a35481113fa2611081f5d18c24 Mon Sep 17 00:00:00 2001 From: Broersma Date: Sat, 2 Dec 2023 16:23:21 +0100 Subject: [PATCH] Fix AsyncClient hangs --- .../usermod_v2_HttpPullLightControl.cpp | 16 +++++++++++++--- .../usermod_v2_HttpPullLightControl.h | 7 +++++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/usermods/usermod_v2_HttpPullLightControl/usermod_v2_HttpPullLightControl.cpp b/usermods/usermod_v2_HttpPullLightControl/usermod_v2_HttpPullLightControl.cpp index 99962611a..854cc2067 100644 --- a/usermods/usermod_v2_HttpPullLightControl/usermod_v2_HttpPullLightControl.cpp +++ b/usermods/usermod_v2_HttpPullLightControl/usermod_v2_HttpPullLightControl.cpp @@ -150,11 +150,18 @@ void HttpPullLightControl::addToConfig(JsonObject& root) { // Do the http request here. Note that we can not do https requests with the AsyncTCP library // We do everything Asynchronous, so all callbacks are defined here void HttpPullLightControl::checkUrl() { + // Extra Inactivity check to see if AsyncCLient hangs + if (client != nullptr && ( millis() - lastActivityTime > inactivityTimeout ) ) { + DEBUG_PRINTLN(F("Inactivity detected, deleting client.")); + delete client; + client = nullptr; + } if (client != nullptr && client->connected()) { DEBUG_PRINTLN(F("We are still connected, do nothing")); // Do nothing, Client is still connected return; } + if (client != nullptr) { // Delete previous client instance if exists, just to prevent any memory leaks DEBUG_PRINTLN(F("Delete previous instances")); @@ -169,6 +176,7 @@ void HttpPullLightControl::checkUrl() { DEBUG_PRINTLN(F("Data received.")); // Cast arg back to the usermod class instance HttpPullLightControl *instance = (HttpPullLightControl *)arg; + instance->lastActivityTime = millis(); // Update lastactivity time when data is received // Convertert to Safe-String char *strData = new char[len + 1]; strncpy(strData, (char*)data, len); @@ -185,19 +193,18 @@ void HttpPullLightControl::checkUrl() { //Set the class-own client pointer to nullptr if its the current client HttpPullLightControl *instance = static_cast(arg); if (instance->client == c) { + delete instance->client; // Delete the client instance instance->client = nullptr; } - // Do not remove client here, it is maintained by AsyncClient }, this); client->onTimeout([](void *arg, AsyncClient *c, uint32_t time) { DEBUG_PRINTLN(F("Timeout")); //Set the class-own client pointer to nullptr if its the current client HttpPullLightControl *instance = static_cast(arg); if (instance->client == c) { - delete instance->client; + delete instance->client; // Delete the client instance instance->client = nullptr; } - // Do not remove client here, it is maintained by AsyncClient }, this); client->onError([](void *arg, AsyncClient *c, int8_t error) { DEBUG_PRINTLN("Connection error occurred!"); @@ -222,6 +229,8 @@ void HttpPullLightControl::checkUrl() { DEBUG_PRINT(host); DEBUG_PRINT(F(" via port ")); DEBUG_PRINTLN((url.startsWith("https")) ? 443 : 80); + // Update lastActivityTime just before sending the request + lastActivityTime = millis(); //Try to connect if (!client->connect(host.c_str(), (url.startsWith("https")) ? 443 : 80)) { DEBUG_PRINTLN(F("Failed to initiate connection.")); @@ -272,6 +281,7 @@ void HttpPullLightControl::handleResponse(String& responseStr) { if (!requestJSONBufferLock(myLockId)) { DEBUG_PRINT(F("ERROR: Can not request JSON Buffer Lock, number: ")); DEBUG_PRINTLN(myLockId); + releaseJSONBufferLock(); // Just release in any case, maybe there was already a buffer lock return; } diff --git a/usermods/usermod_v2_HttpPullLightControl/usermod_v2_HttpPullLightControl.h b/usermods/usermod_v2_HttpPullLightControl/usermod_v2_HttpPullLightControl.h index 5f346f4d7..187b2b091 100644 --- a/usermods/usermod_v2_HttpPullLightControl/usermod_v2_HttpPullLightControl.h +++ b/usermods/usermod_v2_HttpPullLightControl/usermod_v2_HttpPullLightControl.h @@ -60,12 +60,14 @@ private: // Define constants static const uint8_t myLockId = USERMOD_ID_HTTP_PULL_LIGHT_CONTROL ; // Used for the requestJSONBufferLock(id) function - static const int16_t ackTimeout = 10000; // ACK timeout in milliseconds when doing the URL request - static const uint16_t rxTimeout = 10000; // RX timeout in milliseconds when doing the URL request + static const int16_t ackTimeout = 9000; // ACK timeout in milliseconds when doing the URL request + static const uint16_t rxTimeout = 9000; // RX timeout in milliseconds when doing the URL request static const unsigned long FNV_offset_basis = 2166136261; static const unsigned long FNV_prime = 16777619; + static const unsigned long inactivityTimeout = 30000; // When the AsyncClient is inactive (hanging) for this many milliseconds, we kill it unsigned long lastCheck = 0; // Timestamp of last check + unsigned long lastActivityTime = 0; // Time of last activity of AsyncClient String host; // Host extracted from the URL String path; // Path extracted from the URL String uniqueId; // Cached unique ID @@ -96,6 +98,7 @@ public: client->onConnect(nullptr); // Now it is safe to delete the client. delete client; // This is safe even if client is nullptr. + client = nullptr; } } }; \ No newline at end of file