From 929f2900427c995ceb1157e0254ced922cb0030b Mon Sep 17 00:00:00 2001 From: Danila Yudin Date: Sat, 23 May 2026 21:26:13 +0300 Subject: [PATCH] fix: close leaked resource handles (#2040) --- .../converters/JpegToFramebufferConverter.cpp | 22 ++++---------- .../converters/PngToFramebufferConverter.cpp | 22 ++++---------- .../network/CalibreConnectActivity.cpp | 3 ++ .../network/CrossPointWebServerActivity.cpp | 30 ++++++++++++++----- 4 files changed, 37 insertions(+), 40 deletions(-) diff --git a/lib/Epub/Epub/converters/JpegToFramebufferConverter.cpp b/lib/Epub/Epub/converters/JpegToFramebufferConverter.cpp index b0863bb5..a21a59aa 100644 --- a/lib/Epub/Epub/converters/JpegToFramebufferConverter.cpp +++ b/lib/Epub/Epub/converters/JpegToFramebufferConverter.cpp @@ -5,8 +5,10 @@ #include #include #include +#include #include +#include #include #include "DirectPixelWriter.h" @@ -347,16 +349,16 @@ bool JpegToFramebufferConverter::getDimensionsStatic(const std::string& imagePat return false; } - JPEGDEC* jpeg = new (std::nothrow) JPEGDEC(); + std::unique_ptr jpeg(new (std::nothrow) JPEGDEC()); if (!jpeg) { LOG_ERR("JPG", "Failed to allocate JPEG decoder for dimensions"); return false; } int rc = jpeg->open(imagePath.c_str(), jpegOpen, jpegClose, jpegRead, jpegSeek, nullptr); + const ScopedCleanup cleanup{[&jpeg]() { jpeg->close(); }}; if (rc != 1) { LOG_ERR("JPG", "Failed to open JPEG for dimensions (err=%d): %s", jpeg->getLastError(), imagePath.c_str()); - delete jpeg; return false; } @@ -364,8 +366,6 @@ bool JpegToFramebufferConverter::getDimensionsStatic(const std::string& imagePat out.height = jpeg->getHeight(); LOG_DBG("JPG", "Image dimensions: %dx%d", out.width, out.height); - jpeg->close(); - delete jpeg; return true; } @@ -379,7 +379,7 @@ bool JpegToFramebufferConverter::decodeToFramebuffer(const std::string& imagePat return false; } - JPEGDEC* jpeg = new (std::nothrow) JPEGDEC(); + std::unique_ptr jpeg(new (std::nothrow) JPEGDEC()); if (!jpeg) { LOG_ERR("JPG", "Failed to allocate JPEG decoder"); return false; @@ -392,9 +392,9 @@ bool JpegToFramebufferConverter::decodeToFramebuffer(const std::string& imagePat ctx.screenHeight = renderer.getScreenHeight(); int rc = jpeg->open(imagePath.c_str(), jpegOpen, jpegClose, jpegRead, jpegSeek, jpegDrawCallback); + const ScopedCleanup cleanup{[&jpeg]() { jpeg->close(); }}; if (rc != 1) { LOG_ERR("JPG", "Failed to open JPEG (err=%d): %s", jpeg->getLastError(), imagePath.c_str()); - delete jpeg; return false; } @@ -403,14 +403,10 @@ bool JpegToFramebufferConverter::decodeToFramebuffer(const std::string& imagePat if (srcWidth <= 0 || srcHeight <= 0) { LOG_ERR("JPG", "Invalid JPEG dimensions: %dx%d", srcWidth, srcHeight); - jpeg->close(); - delete jpeg; return false; } if (!validateImageDimensions(srcWidth, srcHeight, "JPEG")) { - jpeg->close(); - delete jpeg; return false; } @@ -453,8 +449,6 @@ bool JpegToFramebufferConverter::decodeToFramebuffer(const std::string& imagePat if (destWidth <= 0 || destHeight <= 0) { LOG_ERR("JPG", "Degenerate output dimensions %dx%d for %s, skipping render", destWidth, destHeight, imagePath.c_str()); - jpeg->close(); - delete jpeg; return false; } @@ -490,13 +484,9 @@ bool JpegToFramebufferConverter::decodeToFramebuffer(const std::string& imagePat if (rc != 1) { LOG_ERR("JPG", "Decode failed (rc=%d, lastError=%d)", rc, jpeg->getLastError()); - jpeg->close(); - delete jpeg; return false; } - jpeg->close(); - delete jpeg; LOG_DBG("JPG", "JPEG decoding complete - render time: %lu ms", decodeTime); // Write cache file if caching was enabled diff --git a/lib/Epub/Epub/converters/PngToFramebufferConverter.cpp b/lib/Epub/Epub/converters/PngToFramebufferConverter.cpp index 0cc1616a..245f5c4f 100644 --- a/lib/Epub/Epub/converters/PngToFramebufferConverter.cpp +++ b/lib/Epub/Epub/converters/PngToFramebufferConverter.cpp @@ -4,9 +4,11 @@ #include #include #include +#include #include #include +#include #include #include "DirectPixelWriter.h" @@ -244,7 +246,7 @@ bool PngToFramebufferConverter::getDimensionsStatic(const std::string& imagePath return false; } - PNG* png = new (std::nothrow) PNG(); + std::unique_ptr png(new (std::nothrow) PNG()); if (!png) { LOG_ERR("PNG", "Failed to allocate PNG decoder for dimensions"); return false; @@ -252,18 +254,16 @@ bool PngToFramebufferConverter::getDimensionsStatic(const std::string& imagePath int rc = png->open(imagePath.c_str(), pngOpenWithHandle, pngCloseWithHandle, pngReadWithHandle, pngSeekWithHandle, nullptr); + const ScopedCleanup cleanup{[&png]() { png->close(); }}; if (rc != 0) { LOG_ERR("PNG", "Failed to open PNG for dimensions: %d", rc); - delete png; return false; } out.width = png->getWidth(); out.height = png->getHeight(); - png->close(); - delete png; return true; } @@ -278,7 +278,7 @@ bool PngToFramebufferConverter::decodeToFramebuffer(const std::string& imagePath } // Heap-allocate PNG decoder (~42 KB) - freed at end of function - PNG* png = new (std::nothrow) PNG(); + std::unique_ptr png(new (std::nothrow) PNG()); if (!png) { LOG_ERR("PNG", "Failed to allocate PNG decoder"); return false; @@ -292,15 +292,13 @@ bool PngToFramebufferConverter::decodeToFramebuffer(const std::string& imagePath int rc = png->open(imagePath.c_str(), pngOpenWithHandle, pngCloseWithHandle, pngReadWithHandle, pngSeekWithHandle, pngDrawCallback); + const ScopedCleanup cleanup{[&png]() { png->close(); }}; if (rc != PNG_SUCCESS) { LOG_ERR("PNG", "Failed to open PNG: %d", rc); - delete png; return false; } if (!validateImageDimensions(png->getWidth(), png->getHeight(), "PNG")) { - png->close(); - delete png; return false; } @@ -335,8 +333,6 @@ bool PngToFramebufferConverter::decodeToFramebuffer(const std::string& imagePath "PNG row buffer too small: need %d bytes for width=%d type=%d, configured PNG_MAX_BUFFERED_PIXELS=%d", requiredInternal, ctx.srcWidth, pixelType, PNG_MAX_BUFFERED_PIXELS); LOG_ERR("PNG", "Aborting decode to avoid PNGdec internal buffer overflow"); - png->close(); - delete png; return false; } @@ -349,8 +345,6 @@ bool PngToFramebufferConverter::decodeToFramebuffer(const std::string& imagePath ctx.grayLineBuffer = static_cast(malloc(grayBufSize)); if (!ctx.grayLineBuffer) { LOG_ERR("PNG", "Failed to allocate gray line buffer"); - png->close(); - delete png; return false; } @@ -380,13 +374,9 @@ bool PngToFramebufferConverter::decodeToFramebuffer(const std::string& imagePath if (rc != PNG_SUCCESS) { LOG_ERR("PNG", "Decode failed: %d", rc); - png->close(); - delete png; return false; } - png->close(); - delete png; LOG_DBG("PNG", "PNG decoding complete - render time: %lu ms", decodeTime); // Write cache file if caching was enabled and buffer was allocated diff --git a/src/activities/network/CalibreConnectActivity.cpp b/src/activities/network/CalibreConnectActivity.cpp index ca8b5199..ff89cf27 100644 --- a/src/activities/network/CalibreConnectActivity.cpp +++ b/src/activities/network/CalibreConnectActivity.cpp @@ -52,6 +52,8 @@ void CalibreConnectActivity::onEnter() { void CalibreConnectActivity::onExit() { Activity::onExit(); + MDNS.end(); + if (WiFi.getMode() != WIFI_MODE_NULL) { WiFi.disconnect(false); delay(30); @@ -72,6 +74,7 @@ void CalibreConnectActivity::startWebServer() { state = CalibreConnectState::SERVER_STARTING; requestUpdate(); + MDNS.end(); if (MDNS.begin(HOSTNAME)) { // mDNS is optional for the Calibre plugin but still helpful for users. LOG_DBG("CAL", "mDNS started: http://%s.local/", HOSTNAME); diff --git a/src/activities/network/CrossPointWebServerActivity.cpp b/src/activities/network/CrossPointWebServerActivity.cpp index 44ccfb5b..29737a5a 100644 --- a/src/activities/network/CrossPointWebServerActivity.cpp +++ b/src/activities/network/CrossPointWebServerActivity.cpp @@ -32,6 +32,23 @@ constexpr int QR_CODE_HEIGHT = 198; DNSServer* dnsServer = nullptr; constexpr uint16_t DNS_PORT = 53; +void stopDnsServer() { + if (!dnsServer) return; + + dnsServer->stop(); + delete dnsServer; + dnsServer = nullptr; +} + +void restartMdns(const char* hostname, const char* tag) { + MDNS.end(); + if (MDNS.begin(hostname)) { + LOG_DBG(tag, "mDNS started: http://%s.local/", hostname); + } else { + LOG_DBG(tag, "WARNING: mDNS failed to start"); + } +} + // 0..4 bars from RSSI (dBm), with 3 dBm hysteresis on currentBars to suppress flicker. int barsForRssi(int rssi, int currentBars) { static constexpr int RISE_DBM[] = {-85, -75, -65, -55}; @@ -75,6 +92,8 @@ void CrossPointWebServerActivity::onExit() { LOG_DBG("WEBACT", "Free heap at onExit start: %d bytes", ESP.getFreeHeap()); state = WebServerActivityState::SHUTTING_DOWN; + stopDnsServer(); + MDNS.end(); // Skip reboot if WiFi was never activated (e.g. user backed out of mode selection). if (WiFi.getMode() != WIFI_MODE_NULL) { @@ -151,9 +170,7 @@ void CrossPointWebServerActivity::onWifiSelectionComplete(const bool connected) isApMode = false; // Start mDNS for hostname resolution - if (MDNS.begin(AP_HOSTNAME)) { - LOG_DBG("WEBACT", "mDNS started: http://%s.local/", AP_HOSTNAME); - } + restartMdns(AP_HOSTNAME, "WEBACT"); // Start the web server startWebServer(); @@ -209,14 +226,11 @@ void CrossPointWebServerActivity::startAccessPoint() { LOG_DBG("WEBACT", "IP: %s", connectedIP.c_str()); // Start mDNS for hostname resolution - if (MDNS.begin(AP_HOSTNAME)) { - LOG_DBG("WEBACT", "mDNS started: http://%s.local/", AP_HOSTNAME); - } else { - LOG_DBG("WEBACT", "WARNING: mDNS failed to start"); - } + restartMdns(AP_HOSTNAME, "WEBACT"); // Start DNS server for captive portal behavior // This redirects all DNS queries to our IP, making any domain typed resolve to us + stopDnsServer(); dnsServer = new DNSServer(); dnsServer->setErrorReplyCode(DNSReplyCode::NoError); dnsServer->start(DNS_PORT, "*", apIP);