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.
This commit is contained in:
Ben Meadors
2026-08-14 10:04:57 +00:00
committed by GitHub
parent 5e54262fe1
commit 0ff10318ad
8 changed files with 25 additions and 34 deletions
+4 -5
View File
@@ -1498,14 +1498,13 @@ void loop()
LOG_ERROR("LoRa error detected, recovering"); LOG_ERROR("LoRa error detected, recovering");
router->addInterface(nullptr); router->addInterface(nullptr);
if (portduino_config.lora_spi_dev == "ch341") { if (portduino_config.lora_spi_dev == "ch341") {
if (ch341Hal != nullptr) { if (ch341Hal) {
delete ch341Hal; ch341Hal.reset();
ch341Hal = nullptr;
sleep(3); sleep(3);
} }
try { try {
ch341Hal = new Ch341Hal(0, portduino_config.lora_usb_serial_num, portduino_config.lora_usb_vid, ch341Hal = std::make_unique<Ch341Hal>(0, portduino_config.lora_usb_serial_num, portduino_config.lora_usb_vid,
portduino_config.lora_usb_pid); portduino_config.lora_usb_pid);
} catch (std::exception &e) { } catch (std::exception &e) {
std::cerr << e.what() << std::endl; std::cerr << e.what() << std::endl;
std::cerr << "Could not initialize CH341 device!" << std::endl; std::cerr << "Could not initialize CH341 device!" << std::endl;
+1 -1
View File
@@ -414,7 +414,7 @@ std::unique_ptr<RadioInterface> initLoRa()
LOG_DEBUG("Activate %s radio on SPI port %s", portduino_config.loraModules[portduino_config.lora_module].c_str(), 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()); portduino_config.lora_spi_dev.c_str());
if (portduino_config.lora_spi_dev == "ch341") { 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 { } else {
if (RadioLibHAL != nullptr) { if (RadioLibHAL != nullptr) {
delete RadioLibHAL; delete RadioLibHAL;
+3 -6
View File
@@ -4,23 +4,20 @@
#if HAS_WIFI #if HAS_WIFI
#include "WiFiServerAPI.h" #include "WiFiServerAPI.h"
static WiFiServerPort *apiPort; static std::unique_ptr<WiFiServerPort> apiPort;
void initApiServer(int port) void initApiServer(int port)
{ {
// Start API server on port 4403 // Start API server on port 4403
if (!apiPort) { if (!apiPort) {
apiPort = new WiFiServerPort(port); apiPort = std::make_unique<WiFiServerPort>(port);
LOG_INFO("API server listen on TCP port %d", port); LOG_INFO("API server listen on TCP port %d", port);
apiPort->init(); apiPort->init();
} }
} }
void deInitApiServer() void deInitApiServer()
{ {
if (apiPort) { apiPort.reset();
delete apiPort;
apiPort = nullptr;
}
} }
WiFiServerAPI::WiFiServerAPI(WiFiClient &_client) : ServerAPI(_client) WiFiServerAPI::WiFiServerAPI(WiFiClient &_client) : ServerAPI(_client)
+3 -4
View File
@@ -5,13 +5,13 @@
#include "ethServerAPI.h" #include "ethServerAPI.h"
static ethServerPort *apiPort; static std::unique_ptr<ethServerPort> apiPort;
void initApiServer(int port) void initApiServer(int port)
{ {
// Start API server on port 4403 // Start API server on port 4403
if (!apiPort) { if (!apiPort) {
apiPort = new ethServerPort(port); apiPort = std::make_unique<ethServerPort>(port);
LOG_INFO("API server listening on TCP port %d", port); LOG_INFO("API server listening on TCP port %d", port);
apiPort->init(); apiPort->init();
} }
@@ -21,8 +21,7 @@ void deInitApiServer()
{ {
if (apiPort) { if (apiPort) {
LOG_INFO("Deinit API server"); LOG_INFO("Deinit API server");
delete apiPort; apiPort.reset();
apiPort = nullptr;
} }
} }
+4 -6
View File
@@ -6,6 +6,7 @@
#include "ethApiHandlers.h" #include "ethApiHandlers.h"
#include "ethApiServer.h" #include "ethApiServer.h"
#include <Arduino.h> #include <Arduino.h>
#include <memory>
#ifdef USE_ARDUINO_ETHERNET #ifdef USE_ARDUINO_ETHERNET
#include <Ethernet.h> #include <Ethernet.h>
@@ -20,7 +21,7 @@ static constexpr int32_t ACTIVE_INTERVAL_MS = 20;
static constexpr int32_t MEDIUM_INTERVAL_MS = 100; static constexpr int32_t MEDIUM_INTERVAL_MS = 100;
static constexpr int32_t IDLE_INTERVAL_MS = 500; static constexpr int32_t IDLE_INTERVAL_MS = 500;
static EthernetServer *apiServer = nullptr; static std::unique_ptr<EthernetServer> apiServer;
// Adapter that exposes an EthernetClient through the transport-agnostic // Adapter that exposes an EthernetClient through the transport-agnostic
// IStreamReadWrite interface so the handlers in ethApiHandlers.cpp can drive // 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 // Bind the listener (idempotent - deInitEthApiServer() drops apiServer on a
// W5500 reset, and this rebinds it on the restart path). // W5500 reset, and this rebinds it on the restart path).
if (!apiServer) { if (!apiServer) {
apiServer = new EthernetServer(ETH_API_PORT); apiServer = std::make_unique<EthernetServer>(ETH_API_PORT);
apiServer->begin(); apiServer->begin();
LOG_INFO("ETH API: server listening on TCP port %d (phase 2.0, OSThread @ 20ms)", ETH_API_PORT); 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 // 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 // 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. // next initEthApiServer() from reconnectETH's restart path rebinds TCP/80.
if (apiServer) { apiServer.reset();
delete apiServer;
apiServer = nullptr;
}
} }
#endif // HAS_ETHERNET && HAS_ETHERNET_API #endif // HAS_ETHERNET && HAS_ETHERNET_API
+3 -6
View File
@@ -6,6 +6,7 @@
#include "main.h" #include "main.h"
#include "mesh/http/ContentHelper.h" #include "mesh/http/ContentHelper.h"
#include "mesh/http/WebServer.h" #include "mesh/http/WebServer.h"
#include <memory>
#if HAS_WIFI #if HAS_WIFI
#include "mesh/wifi/WiFiAPClient.h" #include "mesh/wifi/WiFiAPClient.h"
#endif #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 // Actually we do this only for documentary purposes, we know the form is going
// to be multipart/form-data. // to be multipart/form-data.
LOG_DEBUG("Form Upload - Creating body parser reference"); LOG_DEBUG("Form Upload - Creating body parser reference");
HTTPBodyParser *parser; std::unique_ptr<HTTPBodyParser> parser;
std::string contentType = req->getHeader("Content-Type"); std::string contentType = req->getHeader("Content-Type");
// The content type may have additional properties after a semicolon, for example: // 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: // Now, we can decide based on the content type:
if (contentType == "multipart/form-data") { if (contentType == "multipart/form-data") {
LOG_DEBUG("Form Upload - multipart/form-data"); LOG_DEBUG("Form Upload - multipart/form-data");
parser = new HTTPMultipartBodyParser(req); parser.reset(new HTTPMultipartBodyParser(req));
} else { } else {
LOG_DEBUG("Unknown POST Content-Type: %s", contentType.c_str()); LOG_DEBUG("Unknown POST Content-Type: %s", contentType.c_str());
return; return;
@@ -536,7 +537,6 @@ void handleFormUpload(HTTPRequest *req, HTTPResponse *res)
if (name != "file") { if (name != "file") {
LOG_DEBUG("Skip unexpected field"); LOG_DEBUG("Skip unexpected field");
res->println("<p>No file found.</p>"); res->println("<p>No file found.</p>");
delete parser;
return; return;
} }
@@ -544,7 +544,6 @@ void handleFormUpload(HTTPRequest *req, HTTPResponse *res)
if (filename == "") { if (filename == "") {
LOG_DEBUG("Skip unexpected field"); LOG_DEBUG("Skip unexpected field");
res->println("<p>No file found.</p>"); res->println("<p>No file found.</p>");
delete parser;
return; return;
} }
@@ -575,7 +574,6 @@ void handleFormUpload(HTTPRequest *req, HTTPResponse *res)
// enableLoopWDT(); // enableLoopWDT();
delete parser;
return; return;
} }
@@ -596,7 +594,6 @@ void handleFormUpload(HTTPRequest *req, HTTPResponse *res)
res->println("<p>Did not write any file</p>"); res->println("<p>Did not write any file</p>");
} }
res->println("</body></html>"); res->println("</body></html>");
delete parser;
} }
void handleReport(HTTPRequest *req, HTTPResponse *res) void handleReport(HTTPRequest *req, HTTPResponse *res)
+5 -5
View File
@@ -62,7 +62,7 @@ portduino_config_struct portduino_config;
portduino_status_struct portduino_status; portduino_status_struct portduino_status;
std::ofstream traceFile; std::ofstream traceFile;
std::ofstream JSONFile; std::ofstream JSONFile;
Ch341Hal *ch341Hal = nullptr; std::unique_ptr<Ch341Hal> ch341Hal;
char *configPath = nullptr; char *configPath = nullptr;
char *optionMac = nullptr; char *optionMac = nullptr;
bool verboseEnabled = false; bool verboseEnabled = false;
@@ -325,8 +325,8 @@ void portduinoSetup()
{ {
extern void wasm_config_apply(); extern void wasm_config_apply();
wasm_config_apply(); wasm_config_apply();
ch341Hal = ch341Hal = std::make_unique<Ch341Hal>(0, portduino_config.lora_usb_serial_num, portduino_config.lora_usb_vid,
new Ch341Hal(0, portduino_config.lora_usb_serial_num, portduino_config.lora_usb_vid, portduino_config.lora_usb_pid); portduino_config.lora_usb_pid);
} }
return; return;
#endif #endif
@@ -650,8 +650,8 @@ void portduinoSetup()
uint8_t dmac[6] = {0}; uint8_t dmac[6] = {0};
if (portduino_config.lora_spi_dev == "ch341") { if (portduino_config.lora_spi_dev == "ch341") {
try { try {
ch341Hal = new Ch341Hal(0, portduino_config.lora_usb_serial_num, portduino_config.lora_usb_vid, ch341Hal = std::make_unique<Ch341Hal>(0, portduino_config.lora_usb_serial_num, portduino_config.lora_usb_vid,
portduino_config.lora_usb_pid); portduino_config.lora_usb_pid);
} catch (std::exception &e) { } catch (std::exception &e) {
std::cerr << e.what() << std::endl; std::cerr << e.what() << std::endl;
std::cerr << "Could not initialize CH341 device!" << std::endl; std::cerr << "Could not initialize CH341 device!" << std::endl;
+2 -1
View File
@@ -1,6 +1,7 @@
#pragma once #pragma once
#include <fstream> #include <fstream>
#include <map> #include <map>
#include <memory>
#include <unistd.h> #include <unistd.h>
#include <unordered_map> #include <unordered_map>
#include <vector> #include <vector>
@@ -64,7 +65,7 @@ struct pinMapping {
extern std::ofstream traceFile; extern std::ofstream traceFile;
extern std::ofstream JSONFile; extern std::ofstream JSONFile;
extern Ch341Hal *ch341Hal; extern std::unique_ptr<Ch341Hal> ch341Hal;
int initGPIOPin(int pinNum, const std::string &gpioChipname, int line); int initGPIOPin(int pinNum, const std::string &gpioChipname, int line);
bool loadConfig(const char *configPath); bool loadConfig(const char *configPath);
static bool ends_with(std::string_view str, std::string_view suffix); static bool ends_with(std::string_view str, std::string_view suffix);