From ee5a1e28042a7e624a16d93ea68aefaaab4a6262 Mon Sep 17 00:00:00 2001
From: Ivan Grokhotkov <ivan@espressif.com>
Date: Mon, 19 Feb 2018 16:29:34 +0300
Subject: [PATCH] =?UTF-8?q?WiFiClient:=20don=E2=80=99t=20destroy=20ClientC?=
 =?UTF-8?q?ontext=20on=20::stop?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Reported in https://github.com/esp8266/Arduino/issues/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.
---
 libraries/ESP8266WiFi/src/WiFiClient.cpp      |  3 +-
 .../ESP8266WiFi/src/include/ClientContext.h   | 26 ++++-----
 .../test_WiFiClient/test_WiFiClient.ino       | 57 +++++++++++++++++++
 3 files changed, 70 insertions(+), 16 deletions(-)
 create mode 100644 tests/device/test_WiFiClient/test_WiFiClient.ino

diff --git a/libraries/ESP8266WiFi/src/WiFiClient.cpp b/libraries/ESP8266WiFi/src/WiFiClient.cpp
index 84977d87e..30c35fbca 100644
--- a/libraries/ESP8266WiFi/src/WiFiClient.cpp
+++ b/libraries/ESP8266WiFi/src/WiFiClient.cpp
@@ -272,8 +272,7 @@ void WiFiClient::stop()
     if (!_client)
         return;
 
-    _client->unref();
-    _client = 0;
+    _client->close();
 }
 
 uint8_t WiFiClient::connected()
diff --git a/libraries/ESP8266WiFi/src/include/ClientContext.h b/libraries/ESP8266WiFi/src/include/ClientContext.h
index 92e79ee33..9a58beb94 100644
--- a/libraries/ESP8266WiFi/src/include/ClientContext.h
+++ b/libraries/ESP8266WiFi/src/include/ClientContext.h
@@ -107,17 +107,15 @@ public:
 
     void unref()
     {
-        if(this != 0) {
-            DEBUGV(":ur %d\r\n", _refcnt);
-            if(--_refcnt == 0) {
-                discard_received();
-                close();
-                if(_discard_cb) {
-                    _discard_cb(_discard_cb_arg, this);
-                }
-                DEBUGV(":del\r\n");
-                delete this;
+        DEBUGV(":ur %d\r\n", _refcnt);
+        if(--_refcnt == 0) {
+            discard_received();
+            close();
+            if(_discard_cb) {
+                _discard_cb(_discard_cb_arg, this);
             }
+            DEBUGV(":del\r\n");
+            delete this;
         }
     }
 
@@ -131,13 +129,13 @@ public:
         _op_start_time = millis();
         // This delay will be interrupted by esp_schedule in the connect callback
         delay(_timeout_ms);
-        // WiFi may have vanished during the delay (#4078)
-        if (!this || !_pcb) {
-            DEBUGV(":vnsh\r\n");
+        _connect_pending = 0;
+        if (!_pcb) {
+            DEBUGV(":cabrt\r\n");
             return 0;
         }
-        _connect_pending = 0;
         if (state() != ESTABLISHED) {
+            DEBUGV(":ctmo\r\n");
             abort();
             return 0;
         }
diff --git a/tests/device/test_WiFiClient/test_WiFiClient.ino b/tests/device/test_WiFiClient/test_WiFiClient.ino
new file mode 100644
index 000000000..a9cdd5630
--- /dev/null
+++ b/tests/device/test_WiFiClient/test_WiFiClient.ino
@@ -0,0 +1,57 @@
+#include <Arduino.h>
+#include <BSTest.h>
+#include <test_config.h>
+#include <ESP8266WiFi.h>
+#include <Ticker.h>
+
+extern "C" {
+#include "user_interface.h"
+}
+
+BS_ENV_DECLARE();
+
+void setup()
+{
+    Serial.begin(115200);
+    Serial.setDebugOutput(true);
+    WiFi.persistent(false);
+    WiFi.mode(WIFI_STA);
+    WiFi.begin(STA_SSID, STA_PASS);
+    while (WiFi.status() != WL_CONNECTED) {
+        delay(500);
+    }
+    BS_RUN(Serial);
+}
+
+static void stopAll()
+{
+    WiFiClient::stopAll();
+}
+
+static void disconnectWiFI()
+{
+    wifi_station_disconnect();
+}
+
+/* Some IP address that we can try connecting to, and expect a timeout */
+#define UNREACHABLE_IP "192.168.255.255"
+
+TEST_CASE("WiFiClient::stopAll during WiFiClient::connect", "[wificlient]")
+{
+    WiFiClient client;
+    Ticker t;
+    t.once_ms(500, &stopAll);
+    REQUIRE(client.connect(UNREACHABLE_IP, 1024) == 0);
+}
+
+TEST_CASE("WiFi disconnect during WiFiClient::connect", "[wificlient]")
+{
+    WiFiClient client;
+    Ticker t;
+    t.once_ms(500, &disconnectWiFI);
+    REQUIRE(client.connect(UNREACHABLE_IP, 1024) == 0);
+}
+
+void loop()
+{
+}
\ No newline at end of file