Skip to content

Commit 8e6d4c4

Browse files
committed
WiFiClient: don’t destroy ClientContext on ::stop
Reported in #4078. WiFiClient::stopAll, called from a WiFi disconnected event handler, could be called while WiFiClient::connect was in progress. This issue was initially fixed in #4194, by testing `this` pointer for being non-null in ClientContext::connect. This change delegates deletion of ClientContext to WiFiClient destructor. WiFiClient::stop only calls ClientContext::stop, which closes/aborts the connection.
1 parent f3f00ed commit 8e6d4c4

File tree

3 files changed

+70
-16
lines changed

3 files changed

+70
-16
lines changed

libraries/ESP8266WiFi/src/WiFiClient.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,7 @@ void WiFiClient::stop()
272272
if (!_client)
273273
return;
274274

275-
_client->unref();
276-
_client = 0;
275+
_client->close();
277276
}
278277

279278
uint8_t WiFiClient::connected()

libraries/ESP8266WiFi/src/include/ClientContext.h

+12-14
Original file line numberDiff line numberDiff line change
@@ -107,17 +107,15 @@ class ClientContext
107107

108108
void unref()
109109
{
110-
if(this != 0) {
111-
DEBUGV(":ur %d\r\n", _refcnt);
112-
if(--_refcnt == 0) {
113-
discard_received();
114-
close();
115-
if(_discard_cb) {
116-
_discard_cb(_discard_cb_arg, this);
117-
}
118-
DEBUGV(":del\r\n");
119-
delete this;
110+
DEBUGV(":ur %d\r\n", _refcnt);
111+
if(--_refcnt == 0) {
112+
discard_received();
113+
close();
114+
if(_discard_cb) {
115+
_discard_cb(_discard_cb_arg, this);
120116
}
117+
DEBUGV(":del\r\n");
118+
delete this;
121119
}
122120
}
123121

@@ -131,13 +129,13 @@ class ClientContext
131129
_op_start_time = millis();
132130
// This delay will be interrupted by esp_schedule in the connect callback
133131
delay(_timeout_ms);
134-
// WiFi may have vanished during the delay (#4078)
135-
if (!this || !_pcb) {
136-
DEBUGV(":vnsh\r\n");
132+
_connect_pending = 0;
133+
if (!_pcb) {
134+
DEBUGV(":cabrt\r\n");
137135
return 0;
138136
}
139-
_connect_pending = 0;
140137
if (state() != ESTABLISHED) {
138+
DEBUGV(":ctmo\r\n");
141139
abort();
142140
return 0;
143141
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
#include <Arduino.h>
2+
#include <BSTest.h>
3+
#include <test_config.h>
4+
#include <ESP8266WiFi.h>
5+
#include <Ticker.h>
6+
7+
extern "C" {
8+
#include "user_interface.h"
9+
}
10+
11+
BS_ENV_DECLARE();
12+
13+
void setup()
14+
{
15+
Serial.begin(115200);
16+
Serial.setDebugOutput(true);
17+
WiFi.persistent(false);
18+
WiFi.mode(WIFI_STA);
19+
WiFi.begin(STA_SSID, STA_PASS);
20+
while (WiFi.status() != WL_CONNECTED) {
21+
delay(500);
22+
}
23+
BS_RUN(Serial);
24+
}
25+
26+
static void stopAll()
27+
{
28+
WiFiClient::stopAll();
29+
}
30+
31+
static void disconnectWiFI()
32+
{
33+
wifi_station_disconnect();
34+
}
35+
36+
/* Some IP address that we can try connecting to, and expect a timeout */
37+
#define UNREACHABLE_IP "192.168.255.255"
38+
39+
TEST_CASE("WiFiClient::stopAll during WiFiClient::connect", "[wificlient]")
40+
{
41+
WiFiClient client;
42+
Ticker t;
43+
t.once_ms(500, &stopAll);
44+
REQUIRE(client.connect(UNREACHABLE_IP, 1024) == 0);
45+
}
46+
47+
TEST_CASE("WiFi disconnect during WiFiClient::connect", "[wificlient]")
48+
{
49+
WiFiClient client;
50+
Ticker t;
51+
t.once_ms(500, &disconnectWiFI);
52+
REQUIRE(client.connect(UNREACHABLE_IP, 1024) == 0);
53+
}
54+
55+
void loop()
56+
{
57+
}

0 commit comments

Comments
 (0)