Skip to content

Commit 090db26

Browse files
vlastahajekvlastahajek
committed
fix: fixing leaking wifi client
1 parent 8e7b778 commit 090db26

File tree

3 files changed

+73
-52
lines changed

3 files changed

+73
-52
lines changed

‎src/InfluxDbClient.cpp

Lines changed: 57 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -109,13 +109,14 @@ void InfluxDBClient::setConnectionParamsV1(const char *serverUrl, const char *db
109109
}
110110

111111
bool InfluxDBClient::init() {
112-
INFLUXDB_CLIENT_DEBUG("Init\n");
113-
INFLUXDB_CLIENT_DEBUG(" Library version: " INFLUXDB_CLIENT_VERSION "\n");
114-
INFLUXDB_CLIENT_DEBUG(" Server url: %s\n", _serverUrl.c_str());
115-
INFLUXDB_CLIENT_DEBUG(" Org: %s\n", _org.c_str());
116-
INFLUXDB_CLIENT_DEBUG(" Bucket: %s\n", _bucket.c_str());
117-
INFLUXDB_CLIENT_DEBUG(" Token: %s\n", _authToken.c_str());
118-
INFLUXDB_CLIENT_DEBUG(" DB version: %d\n", _dbVersion);
112+
INFLUXDB_CLIENT_DEBUG("[D] Init\n");
113+
INFLUXDB_CLIENT_DEBUG("[D] Library version: " INFLUXDB_CLIENT_VERSION "\n");
114+
INFLUXDB_CLIENT_DEBUG("[D] Server url: %s\n", _serverUrl.c_str());
115+
INFLUXDB_CLIENT_DEBUG("[D] Org: %s\n", _org.c_str());
116+
INFLUXDB_CLIENT_DEBUG("[D] Bucket: %s\n", _bucket.c_str());
117+
INFLUXDB_CLIENT_DEBUG("[D] Token: %s\n", _authToken.c_str());
118+
INFLUXDB_CLIENT_DEBUG("[D] DB version: %d\n", _dbVersion);
119+
INFLUXDB_CLIENT_DEBUG("[D] Connection reuse: %s\n", _httpOptions._connectionReuse?"true":"false");
119120
if(_serverUrl.length() == 0 || (_dbVersion == 2 && (_org.length() == 0 || _bucket.length() == 0 || _authToken.length() == 0))) {
120121
INFLUXDB_CLIENT_DEBUG("[E] Invalid parameters\n");
121122
return false;
@@ -150,9 +151,12 @@ bool InfluxDBClient::init() {
150151
} else {
151152
_wifiClient = new WiFiClient;
152153
}
153-
_httpClient.setReuse(_httpOptions._connectionReuse);
154+
if(!_httpClient) {
155+
_httpClient = new HTTPClient;
156+
}
157+
_httpClient->setReuse(_httpOptions._connectionReuse);
154158

155-
_httpClient.setUserAgent(FPSTR(UserAgent));
159+
_httpClient->setUserAgent(FPSTR(UserAgent));
156160
return true;
157161
}
158162

@@ -192,9 +196,9 @@ bool checkMFLN(BearSSL::WiFiClientSecure *client, String url) {
192196
portS.remove(0, (index + 1)); // remove hostname + :
193197
port = portS.toInt(); // get port
194198
}
195-
INFLUXDB_CLIENT_DEBUG("probeMaxFragmentLength to %s:%d\n", host.c_str(), port);
199+
INFLUXDB_CLIENT_DEBUG("[D] probeMaxFragmentLength to %s:%d\n", host.c_str(), port);
196200
bool mfln = client->probeMaxFragmentLength(host, port, 1024);
197-
INFLUXDB_CLIENT_DEBUG(" MFLN:%s\n", mfln ? "yes" : "no");
201+
INFLUXDB_CLIENT_DEBUG("[D] MFLN:%s\n", mfln ? "yes" : "no");
198202
if (mfln) {
199203
client->setBufferSizes(1024, 1024);
200204
}
@@ -214,7 +218,14 @@ InfluxDBClient::~InfluxDBClient() {
214218
}
215219

216220
void InfluxDBClient::clean() {
217-
_wifiClient = nullptr;
221+
if(_httpClient) {
222+
delete _httpClient;
223+
_httpClient = nullptr;
224+
}
225+
if(_wifiClient) {
226+
delete _wifiClient;
227+
_wifiClient = nullptr;
228+
}
218229
#if defined(ESP8266)
219230
if(_cert) {
220231
delete _cert;
@@ -229,18 +240,18 @@ void InfluxDBClient::clean() {
229240
}
230241

231242
void InfluxDBClient::setUrls() {
232-
INFLUXDB_CLIENT_DEBUG("setUrls\n");
243+
INFLUXDB_CLIENT_DEBUG("[D] setUrls\n");
233244
if(_dbVersion == 2) {
234245
_writeUrl = _serverUrl;
235246
_writeUrl += "/api/v2/write?org=";
236247
_writeUrl += urlEncode(_org.c_str());
237248
_writeUrl += "&bucket=";
238249
_writeUrl += urlEncode(_bucket.c_str());
239-
INFLUXDB_CLIENT_DEBUG(" writeUrl: %s\n", _writeUrl.c_str());
250+
INFLUXDB_CLIENT_DEBUG("[D] writeUrl: %s\n", _writeUrl.c_str());
240251
_queryUrl = _serverUrl;
241252
_queryUrl += "/api/v2/query?org=";
242253
_queryUrl += urlEncode(_org.c_str());
243-
INFLUXDB_CLIENT_DEBUG(" queryUrl: %s\n", _queryUrl.c_str());
254+
INFLUXDB_CLIENT_DEBUG("[D] queryUrl: %s\n", _queryUrl.c_str());
244255
} else {
245256
_writeUrl = _serverUrl;
246257
_writeUrl += "/write?db=";
@@ -256,13 +267,13 @@ void InfluxDBClient::setUrls() {
256267
_queryUrl += "?";
257268
_queryUrl += auth;
258269
}
259-
INFLUXDB_CLIENT_DEBUG(" writeUrl: %s\n", _writeUrl.c_str());
260-
INFLUXDB_CLIENT_DEBUG(" queryUrl: %s\n", _queryUrl.c_str());
270+
INFLUXDB_CLIENT_DEBUG("[D] writeUrl: %s\n", _writeUrl.c_str());
271+
INFLUXDB_CLIENT_DEBUG("[D] queryUrl: %s\n", _queryUrl.c_str());
261272
}
262273
if(_writeOptions._writePrecision != WritePrecision::NoTime) {
263274
_writeUrl += "&precision=";
264275
_writeUrl += precisionToString(_writeOptions._writePrecision, _dbVersion);
265-
INFLUXDB_CLIENT_DEBUG(" writeUrl: %s\n", _writeUrl.c_str());
276+
INFLUXDB_CLIENT_DEBUG("[D] writeUrl: %s\n", _writeUrl.c_str());
266277
}
267278

268279
}
@@ -297,8 +308,11 @@ void InfluxDBClient::setWriteOptions(const WriteOptions & writeOptions) {
297308

298309
void InfluxDBClient::setHTTPOptions(const HTTPOptions & httpOptions) {
299310
_httpOptions = httpOptions;
300-
_httpClient.setReuse(_httpOptions._connectionReuse);
301-
_httpClient.setTimeout(_httpOptions._httpReadTimeout);
311+
if(!_httpClient) {
312+
_httpClient = new HTTPClient;
313+
}
314+
_httpClient->setReuse(_httpOptions._connectionReuse);
315+
_httpClient->setTimeout(_httpOptions._httpReadTimeout);
302316
}
303317

304318
void InfluxDBClient::resetBuffer() {
@@ -312,7 +326,7 @@ void InfluxDBClient::resetBuffer() {
312326
if(_writeBufferSize < 2) {
313327
_writeBufferSize = 2;
314328
}
315-
INFLUXDB_CLIENT_DEBUG("Reset buffer: writeBuffSize: %d\n", _writeBufferSize);
329+
INFLUXDB_CLIENT_DEBUG("[D] Reset buffer: writeBuffSize: %d\n", _writeBufferSize);
316330
_writeBuffer = new Batch*[_writeBufferSize];
317331
for(int i=0;i<_writeBufferSize;i++) {
318332
_writeBuffer[i] = nullptr;
@@ -325,7 +339,7 @@ void InfluxDBClient::resetBuffer() {
325339
void InfluxDBClient::reserveBuffer(int size) {
326340
if(size > _writeBufferSize) {
327341
Batch **newBuffer = new Batch*[size];
328-
INFLUXDB_CLIENT_DEBUG("Resizing buffer from %d to %d\n",_writeBufferSize, size);
342+
INFLUXDB_CLIENT_DEBUG("[D] Resizing buffer from %d to %d\n",_writeBufferSize, size);
329343
for(int i=0;i<_bufferCeiling; i++) {
330344
newBuffer[i] = _writeBuffer[i];
331345
}
@@ -533,29 +547,29 @@ bool InfluxDBClient::validateConnection() {
533547
String url = _serverUrl + (_dbVersion==2?"/health":"/ping?verbose=true");
534548
INFLUXDB_CLIENT_DEBUG("[D] Validating connection to %s\n", url.c_str());
535549

536-
if(!_httpClient.begin(*_wifiClient, url)) {
550+
if(!_httpClient->begin(*_wifiClient, url)) {
537551
INFLUXDB_CLIENT_DEBUG("[E] begin failed\n");
538552
return false;
539553
}
540-
_httpClient.addHeader(F("Accept"), F("application/json"));
554+
_httpClient->addHeader(F("Accept"), F("application/json"));
541555

542-
_lastStatusCode = _httpClient.GET();
556+
_lastStatusCode = _httpClient->GET();
543557

544558
_lastErrorResponse = "";
545559

546560
afterRequest(200, false);
547561

548-
_httpClient.end();
562+
_httpClient->end();
549563

550564
return _lastStatusCode == 200;
551565
}
552566

553567
void InfluxDBClient::beforeRequest() {
554568
if(_authToken.length() > 0) {
555-
_httpClient.addHeader(F("Authorization"), "Token " + _authToken);
569+
_httpClient->addHeader(F("Authorization"), "Token " + _authToken);
556570
}
557571
const char * headerKeys[] = {RetryAfter, TransferEncoding} ;
558-
_httpClient.collectHeaders(headerKeys, 2);
572+
_httpClient->collectHeaders(headerKeys, 2);
559573
}
560574

561575
int InfluxDBClient::postData(const char *data) {
@@ -566,22 +580,22 @@ int InfluxDBClient::postData(const char *data) {
566580
}
567581
if(data) {
568582
INFLUXDB_CLIENT_DEBUG("[D] Writing to %s\n", _writeUrl.c_str());
569-
if(!_httpClient.begin(*_wifiClient, _writeUrl)) {
583+
if(!_httpClient->begin(*_wifiClient, _writeUrl)) {
570584
INFLUXDB_CLIENT_DEBUG("[E] Begin failed\n");
571585
return false;
572586
}
573587
INFLUXDB_CLIENT_DEBUG("[D] Sending:\n%s\n", data);
574588

575-
_httpClient.addHeader(F("Content-Type"), F("text/plain"));
589+
_httpClient->addHeader(F("Content-Type"), F("text/plain"));
576590

577591
beforeRequest();
578592

579-
_lastStatusCode = _httpClient.POST((uint8_t*)data, strlen(data));
593+
_lastStatusCode = _httpClient->POST((uint8_t*)data, strlen(data));
580594

581595
afterRequest(204);
582596

583597

584-
_httpClient.end();
598+
_httpClient->end();
585599
}
586600
return _lastStatusCode;
587601
}
@@ -615,11 +629,11 @@ FluxQueryResult InfluxDBClient::query(String fluxQuery) {
615629
return FluxQueryResult(_lastErrorResponse);
616630
}
617631
INFLUXDB_CLIENT_DEBUG("[D] Query to %s\n", _queryUrl.c_str());
618-
if(!_httpClient.begin(*_wifiClient, _queryUrl)) {
632+
if(!_httpClient->begin(*_wifiClient, _queryUrl)) {
619633
INFLUXDB_CLIENT_DEBUG("[E] begin failed\n");
620634
return FluxQueryResult("");;
621635
}
622-
_httpClient.addHeader(F("Content-Type"), F("application/json"));
636+
_httpClient->addHeader(F("Content-Type"), F("application/json"));
623637

624638
beforeRequest();
625639

@@ -629,22 +643,22 @@ FluxQueryResult InfluxDBClient::query(String fluxQuery) {
629643
body += escapeJSONString(fluxQuery) + "\",";
630644
body += FPSTR(QueryDialect);
631645

632-
_lastStatusCode = _httpClient.POST(body);
646+
_lastStatusCode = _httpClient->POST(body);
633647

634648
afterRequest(200);
635649
if(_lastStatusCode == 200) {
636650
bool chunked = false;
637-
if(_httpClient.hasHeader(TransferEncoding)) {
638-
String header = _httpClient.header(TransferEncoding);
651+
if(_httpClient->hasHeader(TransferEncoding)) {
652+
String header = _httpClient->header(TransferEncoding);
639653
chunked = header.equalsIgnoreCase("chunked");
640654
}
641655
INFLUXDB_CLIENT_DEBUG("[D] chunked: %s\n", chunked?"true":"false");
642-
HttpStreamScanner *scanner = new HttpStreamScanner(&_httpClient, chunked);
656+
HttpStreamScanner *scanner = new HttpStreamScanner(_httpClient, chunked);
643657
CsvReader *reader = new CsvReader(scanner);
644658

645659
return FluxQueryResult(reader);
646660
} else {
647-
_httpClient.end();
661+
_httpClient->end();
648662
return FluxQueryResult(_lastErrorResponse);
649663
}
650664
}
@@ -655,19 +669,19 @@ void InfluxDBClient::afterRequest(int expectedStatusCode, bool modifyLastConnSt
655669
INFLUXDB_CLIENT_DEBUG("[D] HTTP status code - %d\n", _lastStatusCode);
656670
_lastRetryAfter = 0;
657671
if(_lastStatusCode >= 429) { //retryable server errors
658-
if(_httpClient.hasHeader(RetryAfter)) {
659-
_lastRetryAfter = _httpClient.header(RetryAfter).toInt();
672+
if(_httpClient->hasHeader(RetryAfter)) {
673+
_lastRetryAfter = _httpClient->header(RetryAfter).toInt();
660674
INFLUXDB_CLIENT_DEBUG("[D] Reply after - %d\n", _lastRetryAfter);
661675
}
662676
}
663677
}
664678
_lastErrorResponse = "";
665679
if(_lastStatusCode != expectedStatusCode) {
666680
if(_lastStatusCode > 0) {
667-
_lastErrorResponse = _httpClient.getString();
681+
_lastErrorResponse = _httpClient->getString();
668682
INFLUXDB_CLIENT_DEBUG("[D] Response:\n%s\n", _lastErrorResponse.c_str());
669683
} else {
670-
_lastErrorResponse = _httpClient.errorToString(_lastStatusCode);
684+
_lastErrorResponse = _httpClient->errorToString(_lastStatusCode);
671685
INFLUXDB_CLIENT_DEBUG("[E] Error - %s\n", _lastErrorResponse.c_str());
672686
}
673687
}

‎src/InfluxDbClient.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ class InfluxDBClient {
219219
// Server reponse or library error message for last failed request
220220
String _lastErrorResponse;
221221
// Underlying HTTPClient instance
222-
HTTPClient _httpClient;
222+
HTTPClient *_httpClient = nullptr;
223223
// Underlying connection object
224224
WiFiClient *_wifiClient = nullptr;
225225
// Certificate info

‎test/Test.cpp

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -460,15 +460,22 @@ void Test::testHTTPReadTimeout() {
460460
}
461461

462462
void Test::testRepeatedInit() {
463+
// test for validation repeated re-init and not leaking wificlient
463464
TEST_INIT("testRepeatedInit");
464-
InfluxDBClient client;
465-
//waitServer(Test::managementUrl, true);
466-
for(int i = 0; i<20;i++) {
467-
client.setConnectionParams(Test::apiUrl, Test::orgName, Test::bucketName, Test::token);
468-
TEST_ASSERT(client.validateConnection());
469-
}
465+
466+
waitServer(Test::managementUrl, true);
467+
uint32_t startRAM = ESP.getFreeHeap();
468+
do {
469+
InfluxDBClient client;
470+
for(int i = 0; i<20;i++) {
471+
client.setConnectionParams(Test::apiUrl, Test::orgName, Test::bucketName, Test::token);
472+
TEST_ASSERT(client.validateConnection());
473+
}
474+
} while(0);
475+
uint32_t endRAM = ESP.getFreeHeap();
476+
long diff = endRAM-startRAM;
477+
TEST_ASSERTM(diff>-100,String(diff));
470478
TEST_END();
471-
deleteAll(Test::apiUrl);
472479
}
473480

474481
void Test::testRetryOnFailedConnection() {
@@ -865,7 +872,7 @@ void Test::testRetriesOnServerOverload() {
865872
}
866873
}
867874
delete p;
868-
delay(164);
875+
delay(162);
869876
}
870877
TEST_ASSERT(!client.isBufferEmpty());
871878
TEST_ASSERT(client.flushBuffer());

0 commit comments

Comments
 (0)