From 37251c914528318b2b4c86d10dde8fdea8030ad2 Mon Sep 17 00:00:00 2001
From: Srdjan Veljkovic <sveljko@gvs.rs>
Date: Sun, 3 Mar 2019 13:02:25 +0100
Subject: [PATCH] Do not await disconnect forever (#29)

There was a bug in PubSubClient::stop() - a surplus call to
`input_state()`. Also, it now waits for data shorter, not the usual
"full (default) time" . This time is not yet user settable.

Since ESP8266 can, in some weird and as-of-yet-unexplained situations,
take a _long_ time (hours, even) to change the state of the
`WiFiClient` to "not connected" after a `stop()`, we no longer await
this disconnection "forever".  There is now a timeout (for now, not
user-settable) and we give up after a short while. This might lead to
some (probably temporary) leaks, but, we haven't run into problems in
our testing.
---
 PubNubDefs.h       | 60 ++++++++++++++++++++++++++++++----------------
 library.properties |  2 +-
 2 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/PubNubDefs.h b/PubNubDefs.h
index f8d7e00..2a62525 100644
--- a/PubNubDefs.h
+++ b/PubNubDefs.h
@@ -179,9 +179,8 @@ class PubSubClient : public PubNub_BASE_CLIENT {
         }
         /* We are still connected. Read the rest of the stream so that
          * we catch the timetoken. */
-        while (wait_for_data()) {
-            char ch = read();
-            this->_state_input(ch, 0, 0);
+        while (wait_for_data(10)) {
+            read();
         }
         json_enabled = false;
     }
@@ -600,6 +599,17 @@ inline void PubSubClient::_grab_timetoken(uint8_t* nextbuf, size_t nextsize)
     timetoken[new_timetoken_len] = 0;
 }
 
+inline bool await_disconnect(PubNub_BASE_CLIENT& client, unsigned long timeout) {
+    unsigned long    t_start = millis();
+    while (client.connected()) {
+        if (millis() - t_start > timeout * 1000UL) {
+            return false;
+        }
+        delay(10);
+    }
+    return true;
+}
+
 
 inline PubNonSubClient* PubNub::publish(const char* channel,
                                         const char* message,
@@ -669,14 +679,16 @@ inline PubNonSubClient* PubNub::publish(const char* channel,
     case PubNub_BH_ERROR:
         DBGprintln("publish() BH_ERROR");
         client.stop();
-        while (client.connected())
-            delay(10);
+        if (!await_disconnect(client, 10)) {
+            DBGprintln("publish() BH_ERROR: disconnect timeout");
+        }
         return 0;
     case PubNub_BH_TIMEOUT:
         DBGprintln("publish() BH_TIMEOUT");
         client.stop();
-        while (client.connected())
-            delay(10);
+        if (!await_disconnect(client, 10)) {
+            DBGprintln("publish() BH_TIMEOUT: disconnect timeout");
+        }
         return 0;
     }
 }
@@ -726,15 +738,17 @@ inline PubSubClient* PubNub::subscribe(const char* channel, int timeout)
         if (!client.wait_for_data()) {
             DBGprintln("No data received!");
             client.stop();
-            while (client.connected())
-                delay(10);
+            if (!await_disconnect(client, 10)) {
+                DBGprintln("subscribe() no data received: disconnect timeout");
+            }
             return 0;
         }
         if (client.read() != '[') {
             DBGprintln("Unexpected body in subscribe response");
             client.stop();
-            while (client.connected())
-                delay(10);
+            if (!await_disconnect(client, 10)) {
+                DBGprintln("subscribe() unexpected body: disconnect timeout");
+            }
             return 0;
         }
         /* Now return handle to the client for further perusal.
@@ -747,17 +761,21 @@ inline PubSubClient* PubNub::subscribe(const char* channel, int timeout)
     case PubNub_BH_ERROR:
         DBGprintln("subscribe() BH_ERROR");
         client.stop();
-        while (client.connected())
-            delay(10);
+        if (!await_disconnect(client, 10)) {
+            DBGprintln("subscribe() BH_ERROR: disconnect timeout");
+        }
         return 0;
 
     case PubNub_BH_TIMEOUT:
         DBGprintln("subscribe() BH_TIMEOUT");
         client.stop();
         DBGprintln("subscribe() BH_TIMEOUT stopped");
-        while (client.connected())
-            delay(10);
-        DBGprintln("subscribe() BH_TIMEOUT disconnected");
+        if (await_disconnect(client, 10)) {
+            DBGprintln("subscribe() BH_TIMEOUT: disconnected");
+        }
+        else {
+            DBGprintln("subscribe() BH_TIMEOUT: disconnect timeout");
+        }
         return 0;
     }
 }
@@ -790,14 +808,16 @@ inline PubNonSubClient* PubNub::history(const char* channel, int limit, int time
     case PubNub_BH_ERROR:
         DBGprintln("history() BH_ERROR");
         client.stop();
-        while (client.connected())
-            delay(10);
+        if (!await_disconnect(client, 10)) {
+            DBGprintln("history() BH_ERROR: disconnect timeout");
+        }
         return 0;
     case PubNub_BH_TIMEOUT:
         DBGprintln("history() BH_TIMEOUT");
         client.stop();
-        while (client.connected())
-            delay(10);
+        if (!await_disconnect(client, 10)) {
+            DBGprintln("history() BH_TIMEOUT: disconnect timeout");
+        }
         return 0;
     }
 }
diff --git a/library.properties b/library.properties
index 10d7fbe..976950f 100644
--- a/library.properties
+++ b/library.properties
@@ -1,5 +1,5 @@
 name=Pubnub
-version=3.1.1
+version=3.2.0
 author=Vladimir Veljkovic <vlada@pubnub.com>
 maintainer=Vladimir Veljkovic <vlada@pubnub.com>
 sentence=Pubnub SDK for Arduino.