From 0ff10318adbc364e8dad0de31d051e3517ec3cce Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Fri, 14 Aug 2026 10:04:57 +0000 Subject: [PATCH] refactor(net): unique_ptr for connection-lifecycle objects (#11459) - WiFiServerAPI/ethServerAPI apiPort and ethApiServer's listener are create/destroy cycles that repeat across WiFi teardown and W5500 chip resets; the manual delete+null bookkeeping becomes reset(). (ethTlsApiServer's listener is left for a follow-up: that file is already touched by the partial-init fix PR and converting it here would conflict.) - ContentHandler::handleFormUpload held its body parser raw with delete on four separate exit paths of a per-request handler; any future early return was a silent leak. unique_ptr removes all four. - The portduino ch341Hal global becomes unique_ptr. The LoRa-error recovery loop's delete/null/new sequence was correct only by hand-preserved ordering; it becomes reset()/make_unique. RadioLibHAL keeps a non-owning raw pointer, as before. No behavior change. --- src/main.cpp | 9 ++++----- src/mesh/RadioInterface.cpp | 2 +- src/mesh/api/WiFiServerAPI.cpp | 9 +++------ src/mesh/api/ethServerAPI.cpp | 7 +++---- src/mesh/eth/ethApiServer.cpp | 10 ++++------ src/mesh/http/ContentHandler.cpp | 9 +++------ src/platform/portduino/PortduinoGlue.cpp | 10 +++++----- src/platform/portduino/PortduinoGlue.h | 3 ++- 8 files changed, 25 insertions(+), 34 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 6c13515af4..6b192aa049 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1498,14 +1498,13 @@ void loop() LOG_ERROR("LoRa error detected, recovering"); router->addInterface(nullptr); if (portduino_config.lora_spi_dev == "ch341") { - if (ch341Hal != nullptr) { - delete ch341Hal; - ch341Hal = nullptr; + if (ch341Hal) { + ch341Hal.reset(); sleep(3); } try { - ch341Hal = new Ch341Hal(0, portduino_config.lora_usb_serial_num, portduino_config.lora_usb_vid, - portduino_config.lora_usb_pid); + ch341Hal = std::make_unique(0, portduino_config.lora_usb_serial_num, portduino_config.lora_usb_vid, + portduino_config.lora_usb_pid); } catch (std::exception &e) { std::cerr << e.what() << std::endl; std::cerr << "Could not initialize CH341 device!" << std::endl; diff --git a/src/mesh/RadioInterface.cpp b/src/mesh/RadioInterface.cpp index 36a35ac6b9..58bd498c58 100644 --- a/src/mesh/RadioInterface.cpp +++ b/src/mesh/RadioInterface.cpp @@ -414,7 +414,7 @@ std::unique_ptr initLoRa() LOG_DEBUG("Activate %s radio on SPI port %s", portduino_config.loraModules[portduino_config.lora_module].c_str(), portduino_config.lora_spi_dev.c_str()); if (portduino_config.lora_spi_dev == "ch341") { - RadioLibHAL = ch341Hal; + RadioLibHAL = ch341Hal.get(); // non-owning: the ch341 HAL stays owned by the global unique_ptr } else { if (RadioLibHAL != nullptr) { delete RadioLibHAL; diff --git a/src/mesh/api/WiFiServerAPI.cpp b/src/mesh/api/WiFiServerAPI.cpp index 4d729f5c71..8b46a5725f 100644 --- a/src/mesh/api/WiFiServerAPI.cpp +++ b/src/mesh/api/WiFiServerAPI.cpp @@ -4,23 +4,20 @@ #if HAS_WIFI #include "WiFiServerAPI.h" -static WiFiServerPort *apiPort; +static std::unique_ptr apiPort; void initApiServer(int port) { // Start API server on port 4403 if (!apiPort) { - apiPort = new WiFiServerPort(port); + apiPort = std::make_unique(port); LOG_INFO("API server listen on TCP port %d", port); apiPort->init(); } } void deInitApiServer() { - if (apiPort) { - delete apiPort; - apiPort = nullptr; - } + apiPort.reset(); } WiFiServerAPI::WiFiServerAPI(WiFiClient &_client) : ServerAPI(_client) diff --git a/src/mesh/api/ethServerAPI.cpp b/src/mesh/api/ethServerAPI.cpp index c75d53ff7c..953c9921f0 100644 --- a/src/mesh/api/ethServerAPI.cpp +++ b/src/mesh/api/ethServerAPI.cpp @@ -5,13 +5,13 @@ #include "ethServerAPI.h" -static ethServerPort *apiPort; +static std::unique_ptr apiPort; void initApiServer(int port) { // Start API server on port 4403 if (!apiPort) { - apiPort = new ethServerPort(port); + apiPort = std::make_unique(port); LOG_INFO("API server listening on TCP port %d", port); apiPort->init(); } @@ -21,8 +21,7 @@ void deInitApiServer() { if (apiPort) { LOG_INFO("Deinit API server"); - delete apiPort; - apiPort = nullptr; + apiPort.reset(); } } diff --git a/src/mesh/eth/ethApiServer.cpp b/src/mesh/eth/ethApiServer.cpp index c7f1df6105..c27d97fe37 100644 --- a/src/mesh/eth/ethApiServer.cpp +++ b/src/mesh/eth/ethApiServer.cpp @@ -6,6 +6,7 @@ #include "ethApiHandlers.h" #include "ethApiServer.h" #include +#include #ifdef USE_ARDUINO_ETHERNET #include @@ -20,7 +21,7 @@ static constexpr int32_t ACTIVE_INTERVAL_MS = 20; static constexpr int32_t MEDIUM_INTERVAL_MS = 100; static constexpr int32_t IDLE_INTERVAL_MS = 500; -static EthernetServer *apiServer = nullptr; +static std::unique_ptr apiServer; // Adapter that exposes an EthernetClient through the transport-agnostic // IStreamReadWrite interface so the handlers in ethApiHandlers.cpp can drive @@ -86,7 +87,7 @@ void initEthApiServer() // Bind the listener (idempotent - deInitEthApiServer() drops apiServer on a // W5500 reset, and this rebinds it on the restart path). if (!apiServer) { - apiServer = new EthernetServer(ETH_API_PORT); + apiServer = std::make_unique(ETH_API_PORT); apiServer->begin(); LOG_INFO("ETH API: server listening on TCP port %d (phase 2.0, OSThread @ 20ms)", ETH_API_PORT); } @@ -103,10 +104,7 @@ void deInitEthApiServer() // A W5500 chip reset wipes the hardware socket table, so the listener is now // bound to a dead socket. Drop it (the worker stays alive and idles) so the // next initEthApiServer() from reconnectETH's restart path rebinds TCP/80. - if (apiServer) { - delete apiServer; - apiServer = nullptr; - } + apiServer.reset(); } #endif // HAS_ETHERNET && HAS_ETHERNET_API diff --git a/src/mesh/http/ContentHandler.cpp b/src/mesh/http/ContentHandler.cpp index d6c4904b74..ad0b9a84c3 100644 --- a/src/mesh/http/ContentHandler.cpp +++ b/src/mesh/http/ContentHandler.cpp @@ -6,6 +6,7 @@ #include "main.h" #include "mesh/http/ContentHelper.h" #include "mesh/http/WebServer.h" +#include #if HAS_WIFI #include "mesh/wifi/WiFiAPClient.h" #endif @@ -484,7 +485,7 @@ void handleFormUpload(HTTPRequest *req, HTTPResponse *res) // Actually we do this only for documentary purposes, we know the form is going // to be multipart/form-data. LOG_DEBUG("Form Upload - Creating body parser reference"); - HTTPBodyParser *parser; + std::unique_ptr parser; std::string contentType = req->getHeader("Content-Type"); // The content type may have additional properties after a semicolon, for example: @@ -500,7 +501,7 @@ void handleFormUpload(HTTPRequest *req, HTTPResponse *res) // Now, we can decide based on the content type: if (contentType == "multipart/form-data") { LOG_DEBUG("Form Upload - multipart/form-data"); - parser = new HTTPMultipartBodyParser(req); + parser.reset(new HTTPMultipartBodyParser(req)); } else { LOG_DEBUG("Unknown POST Content-Type: %s", contentType.c_str()); return; @@ -536,7 +537,6 @@ void handleFormUpload(HTTPRequest *req, HTTPResponse *res) if (name != "file") { LOG_DEBUG("Skip unexpected field"); res->println("

No file found.

"); - delete parser; return; } @@ -544,7 +544,6 @@ void handleFormUpload(HTTPRequest *req, HTTPResponse *res) if (filename == "") { LOG_DEBUG("Skip unexpected field"); res->println("

No file found.

"); - delete parser; return; } @@ -575,7 +574,6 @@ void handleFormUpload(HTTPRequest *req, HTTPResponse *res) // enableLoopWDT(); - delete parser; return; } @@ -596,7 +594,6 @@ void handleFormUpload(HTTPRequest *req, HTTPResponse *res) res->println("

Did not write any file

"); } res->println(""); - delete parser; } void handleReport(HTTPRequest *req, HTTPResponse *res) diff --git a/src/platform/portduino/PortduinoGlue.cpp b/src/platform/portduino/PortduinoGlue.cpp index df977a028e..7ac778e3c4 100644 --- a/src/platform/portduino/PortduinoGlue.cpp +++ b/src/platform/portduino/PortduinoGlue.cpp @@ -62,7 +62,7 @@ portduino_config_struct portduino_config; portduino_status_struct portduino_status; std::ofstream traceFile; std::ofstream JSONFile; -Ch341Hal *ch341Hal = nullptr; +std::unique_ptr ch341Hal; char *configPath = nullptr; char *optionMac = nullptr; bool verboseEnabled = false; @@ -325,8 +325,8 @@ void portduinoSetup() { extern void wasm_config_apply(); wasm_config_apply(); - ch341Hal = - new Ch341Hal(0, portduino_config.lora_usb_serial_num, portduino_config.lora_usb_vid, portduino_config.lora_usb_pid); + ch341Hal = std::make_unique(0, portduino_config.lora_usb_serial_num, portduino_config.lora_usb_vid, + portduino_config.lora_usb_pid); } return; #endif @@ -650,8 +650,8 @@ void portduinoSetup() uint8_t dmac[6] = {0}; if (portduino_config.lora_spi_dev == "ch341") { try { - ch341Hal = new Ch341Hal(0, portduino_config.lora_usb_serial_num, portduino_config.lora_usb_vid, - portduino_config.lora_usb_pid); + ch341Hal = std::make_unique(0, portduino_config.lora_usb_serial_num, portduino_config.lora_usb_vid, + portduino_config.lora_usb_pid); } catch (std::exception &e) { std::cerr << e.what() << std::endl; std::cerr << "Could not initialize CH341 device!" << std::endl; diff --git a/src/platform/portduino/PortduinoGlue.h b/src/platform/portduino/PortduinoGlue.h index a6797c2923..2072455671 100644 --- a/src/platform/portduino/PortduinoGlue.h +++ b/src/platform/portduino/PortduinoGlue.h @@ -1,6 +1,7 @@ #pragma once #include #include +#include #include #include #include @@ -64,7 +65,7 @@ struct pinMapping { extern std::ofstream traceFile; extern std::ofstream JSONFile; -extern Ch341Hal *ch341Hal; +extern std::unique_ptr ch341Hal; int initGPIOPin(int pinNum, const std::string &gpioChipname, int line); bool loadConfig(const char *configPath); static bool ends_with(std::string_view str, std::string_view suffix);