Vpn stop management issues (#572)

* Fixed error, when stopVPN method in ManagementThread returned true regardless of actual stop signal have been sent.

As result - management stopped to send any state updates to listeners because of mShuttingDown flag.

* Fixed error, when endVpnService method didn't actually stop vpn thread because of missing thread interruption handling.

3 files changed, 30 insertions, 10 deletions

diff --git a/main/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java b/main/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java
index aa75dd3e..03866962 100644
--- a/main/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java
+++ b/main/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java
@@ -515,6 +515,10 @@ public class OpenVPNService extends VpnService implements StateListener, Callbac
             }
         }
 
+        forceStopOpenVpnProcess();
+    }
+
+    public void forceStopOpenVpnProcess() {
         synchronized (mProcessLock) {
             if (mProcessThread != null) {
                 mProcessThread.interrupt();
diff --git a/main/src/main/java/de/blinkt/openvpn/core/OpenVPNThread.java b/main/src/main/java/de/blinkt/openvpn/core/OpenVPNThread.java
index 59bcca37..b2774861 100644
--- a/main/src/main/java/de/blinkt/openvpn/core/OpenVPNThread.java
+++ b/main/src/main/java/de/blinkt/openvpn/core/OpenVPNThread.java
@@ -180,10 +180,12 @@ public class OpenVPNThread implements Runnable {
                 } else {

                     VpnStatus.logInfo("P:" + logline);

                 }

-            }

-

 

-        } catch (IOException e) {

+                if (Thread.interrupted()) {

+                    throw new InterruptedException("OpenVpn process was killed form java code");

+                }

+            }

+        } catch (InterruptedException | IOException e) {

             VpnStatus.logException("Error reading from output of OpenVPN process", e);

             stopProcess();

         }

diff --git a/main/src/main/java/de/blinkt/openvpn/core/OpenVpnManagementThread.java b/main/src/main/java/de/blinkt/openvpn/core/OpenVpnManagementThread.java
index 16a03394..bb0b2221 100644
--- a/main/src/main/java/de/blinkt/openvpn/core/OpenVpnManagementThread.java
+++ b/main/src/main/java/de/blinkt/openvpn/core/OpenVpnManagementThread.java
@@ -105,15 +105,21 @@ public class OpenVpnManagementThread implements Runnable, OpenVPNManagement {
 

     }

 

-    public void managmentCommand(String cmd) {

+    /**

+     * @param cmd command to write to management socket

+     * @return true if command have been sent

+     */

+    public boolean managmentCommand(String cmd) {

         try {

             if (mSocket != null && mSocket.getOutputStream() != null) {

                 mSocket.getOutputStream().write(cmd.getBytes());

                 mSocket.getOutputStream().flush();

+                return true;

             }

         } catch (IOException e) {

             // Ignore socket stack traces

         }

+        return false;

     }

 

 

@@ -134,8 +140,12 @@ public class OpenVpnManagementThread implements Runnable, OpenVPNManagement {
 

 

             // Close the management socket after client connected

+            try {

+                mServerSocket.close();

+            } catch (IOException e){

+                VpnStatus.logException(e);

+            }

 

-            mServerSocket.close();

             // Closing one of the two sockets also closes the other

             //mServerSocketLocal.close();

 

@@ -164,7 +174,8 @@ public class OpenVpnManagementThread implements Runnable, OpenVPNManagement {
 

             }

         } catch (IOException e) {

-            VpnStatus.logDebug(R.string.management_socket_closed, e.getLocalizedMessage());

+            if (!e.getMessage().equals("socket closed") && !e.getMessage().equals("Connection reset by peer"))

+                VpnStatus.logException(e);

         }

         synchronized (active) {

             active.remove(this);

@@ -577,8 +588,7 @@ public class OpenVpnManagementThread implements Runnable, OpenVPNManagement {
         synchronized (active) {

             boolean sendCMD = false;

             for (OpenVpnManagementThread mt : active) {

-                mt.managmentCommand("signal SIGINT\n");

-                sendCMD = true;

+                sendCMD = mt.managmentCommand("signal SIGINT\n");

                 try {

                     if (mt.mSocket != null)

                         mt.mSocket.close();

@@ -649,8 +659,12 @@ public class OpenVpnManagementThread implements Runnable, OpenVPNManagement {
 

     @Override

     public boolean stopVPN(boolean replaceConnection) {

-        mShuttingDown = true;

-        return stopOpenVPN();

+        boolean stopSucceed = stopOpenVPN();

+        if (stopSucceed) {

+            mShuttingDown = true;

+

+        }

+        return stopSucceed;

     }

 

 }