From dab4215a9a447de8c69d670964c964f8897753b8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Teppo=20J=C3=A4rvelin?= <teppo.jarvelin@arm.com>
Date: Thu, 7 Jun 2018 12:47:01 +0300
Subject: [PATCH 1/2] Fixed ppp disconnect hangs when connection failure.

---
 .../FEATURE_LWIP/lwip-interface/ppp_lwip.cpp  | 45 +++++++++++++------
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/features/FEATURE_LWIP/lwip-interface/ppp_lwip.cpp b/features/FEATURE_LWIP/lwip-interface/ppp_lwip.cpp
index c05e6c9e9e..6989073112 100644
--- a/features/FEATURE_LWIP/lwip-interface/ppp_lwip.cpp
+++ b/features/FEATURE_LWIP/lwip-interface/ppp_lwip.cpp
@@ -295,20 +295,18 @@ extern "C" err_t ppp_lwip_connect(void *pcb)
 
 extern "C" err_t ppp_lwip_disconnect(void *pcb)
 {
-    err_t ret = ppp_close(my_ppp_pcb, 0);
-    if (ret != ERR_OK) {
-        return ret;
+    if (ppp_active) {
+        err_t ret = ppp_close(my_ppp_pcb, 0);
+        if (ret != ERR_OK) {
+            return ret;
+        }
+
+        /* close call made, now let's catch the response in the status callback */
+        sys_arch_sem_wait(&ppp_close_sem, 0);
+        ppp_active = false;
     }
 
-    /* close call made, now let's catch the response in the status callback */
-    sys_arch_sem_wait(&ppp_close_sem, 0);
-
-    /* Detach callbacks, and put handle back to default blocking mode */
-    my_stream->sigio(Callback<void()>());
-    my_stream->set_blocking(true);
-    my_stream = NULL;
-
-    return ret;
+    return ERR_OK;
 }
 
 extern "C" nsapi_error_t ppp_lwip_if_init(void *pcb, struct netif *netif, const nsapi_ip_stack_t stack)
@@ -373,6 +371,9 @@ nsapi_error_t nsapi_ppp_connect(FileHandle *stream, Callback<void(nsapi_event_t,
         retcode = lwip._add_ppp_interface(stream, true, stack, &my_interface);
         if (retcode != NSAPI_ERROR_OK) {
             my_interface = NULL;
+            my_stream->set_blocking(true);
+            my_stream = NULL;
+            connection_status_cb = NULL;
             return retcode;
         }
     }
@@ -381,6 +382,12 @@ nsapi_error_t nsapi_ppp_connect(FileHandle *stream, Callback<void(nsapi_event_t,
     // attach deferred until ppp_lwip_connect, called from mbed_lwip_bringup
     retcode = my_interface->bringup(false, NULL, NULL, NULL, stack, blocking_connect);
 
+    if (retcode != NSAPI_ERROR_OK) {
+        connection_status_cb = NULL;
+        my_stream->set_blocking(true);
+        my_stream = NULL;
+    }
+
     if (retcode != NSAPI_ERROR_OK && connect_error_code != NSAPI_ERROR_OK) {
         return connect_error_code;
     } else {
@@ -390,7 +397,19 @@ nsapi_error_t nsapi_ppp_connect(FileHandle *stream, Callback<void(nsapi_event_t,
 
 nsapi_error_t nsapi_ppp_disconnect(FileHandle *stream)
 {
-    return my_interface->bringdown();
+    if (my_stream != stream) {
+        return NSAPI_ERROR_NO_CONNECTION;
+    }
+
+    nsapi_error_t retcode = my_interface->bringdown();
+
+    connection_status_cb = NULL;
+    /* Detach callbacks, and put handle back to default blocking mode */
+    my_stream->sigio(NULL);
+    my_stream->set_blocking(true);
+    my_stream = NULL;
+
+    return retcode;
 }
 
 NetworkStack *nsapi_ppp_get_stack()

From b35dc6a5822185395b84a0848d14afbafd9a41ac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Teppo=20J=C3=A4rvelin?= <teppo.jarvelin@arm.com>
Date: Fri, 8 Jun 2018 08:49:28 +0300
Subject: [PATCH 2/2] Review fix: - set ppp_active false if close fails in ppp
 disconnect. - unset sigio in ppp disconnect - take ownership of filehandle in
 CellularNetwork::disconnect even in case of failure

---
 features/FEATURE_LWIP/lwip-interface/ppp_lwip.cpp | 15 +++++++--------
 .../cellular/framework/AT/AT_CellularNetwork.cpp  |  8 +++-----
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/features/FEATURE_LWIP/lwip-interface/ppp_lwip.cpp b/features/FEATURE_LWIP/lwip-interface/ppp_lwip.cpp
index 6989073112..97fe9feb91 100644
--- a/features/FEATURE_LWIP/lwip-interface/ppp_lwip.cpp
+++ b/features/FEATURE_LWIP/lwip-interface/ppp_lwip.cpp
@@ -295,18 +295,16 @@ extern "C" err_t ppp_lwip_connect(void *pcb)
 
 extern "C" err_t ppp_lwip_disconnect(void *pcb)
 {
+    err_t ret = ERR_OK;
     if (ppp_active) {
-        err_t ret = ppp_close(my_ppp_pcb, 0);
-        if (ret != ERR_OK) {
-            return ret;
+        ret = ppp_close(my_ppp_pcb, 0);
+        if (ret == ERR_OK) {
+            /* close call made, now let's catch the response in the status callback */
+            sys_arch_sem_wait(&ppp_close_sem, 0);
         }
-
-        /* close call made, now let's catch the response in the status callback */
-        sys_arch_sem_wait(&ppp_close_sem, 0);
         ppp_active = false;
     }
-
-    return ERR_OK;
+    return ret;
 }
 
 extern "C" nsapi_error_t ppp_lwip_if_init(void *pcb, struct netif *netif, const nsapi_ip_stack_t stack)
@@ -384,6 +382,7 @@ nsapi_error_t nsapi_ppp_connect(FileHandle *stream, Callback<void(nsapi_event_t,
 
     if (retcode != NSAPI_ERROR_OK) {
         connection_status_cb = NULL;
+        my_stream->sigio(NULL);
         my_stream->set_blocking(true);
         my_stream = NULL;
     }
diff --git a/features/cellular/framework/AT/AT_CellularNetwork.cpp b/features/cellular/framework/AT/AT_CellularNetwork.cpp
index ff0d9876f6..8710c02b84 100644
--- a/features/cellular/framework/AT/AT_CellularNetwork.cpp
+++ b/features/cellular/framework/AT/AT_CellularNetwork.cpp
@@ -374,11 +374,9 @@ nsapi_error_t AT_CellularNetwork::disconnect()
     nsapi_error_t err = nsapi_ppp_disconnect(_at.get_file_handle());
     // after ppp disconnect if we wan't to use same at handler we need to set filehandle again to athandler so it
     // will set the correct sigio and nonblocking
-    if (err == NSAPI_ERROR_OK) {
-        _at.lock();
-        _at.set_file_handle(_at.get_file_handle());
-        _at.unlock();
-    }
+    _at.lock();
+    _at.set_file_handle(_at.get_file_handle());
+    _at.unlock();
     return err;
 #else
     _at.lock();