[refactor] move terminate_or_kill to linux launcher

3 files changed, 92 insertions, 85 deletions

diff --git a/src/leap/bitmask/vpn/launchers/linux.py b/src/leap/bitmask/vpn/launchers/linux.py
index 38b9e417..052040dc 100644
--- a/src/leap/bitmask/vpn/launchers/linux.py
+++ b/src/leap/bitmask/vpn/launchers/linux.py
@@ -33,6 +33,11 @@ from leap.bitmask.vpn.privilege import LinuxPolicyChecker
 from leap.bitmask.vpn.management import ManagementProtocol
 from leap.bitmask.vpn.launcher import VPNLauncher
 
+
+TERMINATE_MAXTRIES = 10
+TERMINATE_WAIT = 1  # secs
+RESTART_WAIT = 2  # secs
+
 log = Logger()
 
 
@@ -144,6 +149,42 @@ class LinuxVPNLauncher(VPNLauncher):
 
         return command
 
+    def terminate_or_kill(self, terminatefun, killfun, proc):
+        terminatefun()
+
+        # we trigger a countdown to be unpolite
+        # if strictly needed.
+        d = defer.Deferred()
+        reactor.callLater(
+            TERMINATE_WAIT, self._wait_and_kill, killfun, proc, d)
+        return d
+
+    def _wait_and_kill(self, killfun, proc, deferred, tries=0):
+        """
+        Check if the process is still alive, and call the killfun
+        after waiting several times during a timeout period.
+
+        :param tries: counter of tries, used in recursion
+        :type tries: int
+        """
+        if tries < TERMINATE_MAXTRIES:
+            if proc.transport.pid is None:
+                deferred.callback(True)
+                return
+            else:
+                self.log.debug('Process did not die, waiting...')
+
+            tries += 1
+            reactor.callLater(
+                TERMINATE_WAIT,
+                self._wait_and_kill, killfun, proc, deferred, tries)
+            return
+
+        # after running out of patience, we try a killProcess
+        d = killfun()
+        d.addCallback(lambda _: deferred.callback(True))
+        return d
+
     def kill_previous_openvpn(kls):
         """
         Checks if VPN is already running and tries to stop it.
diff --git a/src/leap/bitmask/vpn/process.py b/src/leap/bitmask/vpn/process.py
index 0ba4b85a..48d3f1ad 100644
--- a/src/leap/bitmask/vpn/process.py
+++ b/src/leap/bitmask/vpn/process.py
@@ -22,11 +22,10 @@ A custom processProtocol launches the VPNProcess and connects to its management
 interface.
 """
 
+import os
 import shutil
 import sys
 
-import psutil
-
 from twisted.internet import protocol, reactor, defer
 from twisted.internet import error as internet_error
 from twisted.internet.endpoints import clientFromString, connectProtocol
@@ -53,7 +52,8 @@ class _VPNProcess(protocol.ProcessProtocol):
     log = Logger()
 
     # HACK - reactor is expected to set this up when the process is spawned.
-    # should try to get it from within this class.
+    # should try to get it from the management protocol instead.
+    # XXX or, at least, we can check if they match.
     pid = None
 
     # TODO do we really need the vpnconfig/providerconfig objects in here???
@@ -82,6 +82,7 @@ class _VPNProcess(protocol.ProcessProtocol):
                 reactor, b"unix:path=%s" % socket_host)
         else:
             raise ValueError('tcp endpoint not configured')
+
         self._vpnconfig = vpnconfig
         self._providerconfig = providerconfig
         self._launcher = get_vpn_launcher()
@@ -115,7 +116,6 @@ class _VPNProcess(protocol.ProcessProtocol):
         self._d.addErrback(self.log.error)
 
     def connectionMade(self):
-        self.aborted = False
         # TODO cut this wait time when retries are done
         reactor.callLater(0.5, self._connect_to_management)
 
@@ -135,6 +135,9 @@ class _VPNProcess(protocol.ProcessProtocol):
             # TODO: need to exit properly!
             status, errmsg = 'off', None
 
+        # TODO ---- propagate this status upwards!!
+        # XXX do something with status
+
     def processEnded(self, reason):
         """
         Called when the child process exits and all file descriptors associated
@@ -226,6 +229,11 @@ class _VPNProcess(protocol.ProcessProtocol):
         except internet_error.ProcessExitedAlready:
             self.log.debug('Process Exited Already')
 
+    def terminate_or_kill(self):
+        # XXX this returns a deferred
+        return self._launcher.terminate_or_kill(
+            self.terminate, self.kill, self)
+
 
 if IS_LINUX:
 
diff --git a/src/leap/bitmask/vpn/tunnel.py b/src/leap/bitmask/vpn/tunnel.py
index a83704f6..d05a50e2 100644
--- a/src/leap/bitmask/vpn/tunnel.py
+++ b/src/leap/bitmask/vpn/tunnel.py
@@ -26,13 +26,10 @@ from twisted.internet import reactor, defer
 from twisted.logger import Logger
 
 from ._config import _TempVPNConfig, _TempProviderConfig
-from .constants import IS_WIN, IS_LINUX
+from .constants import IS_WIN
 from .process import VPNProcess
 
 
-# TODO gateway selection should be done in this class.
-
-
 # TODO ----------------- refactor --------------------
 # [ ] register change state listener
 # emit_async(catalog.VPN_STATUS_CHANGED)
@@ -41,6 +38,7 @@ from .process import VPNProcess
 #    'Network is unreachable (code=101)',),
 # 'PROCESS_RESTART_TLS': (
 #    "SIGTERM[soft,tls-error]",),
+# TODO ----------------- refactor --------------------
 
 
 class ConfiguredTunnel(object):
@@ -58,10 +56,6 @@ class ConfiguredTunnel(object):
     demand.
     """
 
-    TERMINATE_MAXTRIES = 10
-    TERMINATE_WAIT = 1  # secs
-    RESTART_WAIT = 2  # secs
-
     log = Logger()
 
     def __init__(self, provider, remotes, cert_path, key_path, ca_path,
@@ -86,7 +80,7 @@ class ConfiguredTunnel(object):
         return self._start_vpn()
 
     def stop(self):
-        return self._stop_vpn(shutdown=False, restart=False)
+        return self._stop_vpn(restart=False)
 
     #  status
 
@@ -107,116 +101,80 @@ class ConfiguredTunnel(object):
         args = [self._vpnconfig, self._providerconfig, self._host,
                 self._port]
         kwargs = {'openvpn_verb': 4, 'remotes': self._remotes,
-                  'restartfun': self.restart}
+                  'restartfun': self._restart_vpn}
         vpnproc = VPNProcess(*args, **kwargs)
+        self._vpnproc = vpnproc
 
+        self._start_pre_up(vpnproc)
+        cmd = self.__start_get_cmd(vpnproc)
+        # XXX this should be a deferred
+        running = self.__start_spawn_proc(vpnproc, cmd)
+        if running:
+            vpnproc.pid = running.pid
+            return True
+        else:
+            return False
+
+    def __start_pre_up(self, proc):
         try:
-            vpnproc.preUp()
-        except Exception as e:
-            self.log.error('Error on vpn pre-up {0!r}'.format(e))
+            proc.preUp()
+        except Exception as exc:
+            self.log.error('Error on vpn pre-up {0!r}'.format(exc))
             raise
+
+    def __start_get_cmd(self, proc):
         try:
-            cmd = vpnproc.getCommand()
-        except Exception as e:
+            cmd = proc.getCommand()
+        except Exception as exc:
             self.log.error(
-                'Error while getting vpn command... {0!r}'.format(e))
+                'Error while getting vpn command... {0!r}'.format(exc))
             raise
+        return cmd
 
+    def __start_spawn_proc(self, proc, cmd):
         env = os.environ
         try:
-            runningproc = reactor.spawnProcess(vpnproc, cmd[0], cmd, env)
+            running_p = reactor.spawnProcess(proc, cmd[0], cmd, env)
         except Exception as e:
             self.log.error(
                 'Error while spawning vpn process... {0!r}'.format(e))
             return False
-
-        # TODO get pid from management instead
-        vpnproc.pid = runningproc.pid
-        self._vpnproc = vpnproc
-        return True
+        return running_p
 
     @defer.inlineCallbacks
     def _restart_vpn(self):
-        yield self.stop(shutdown=False, restart=True)
+        yield self.stop(restart=True)
         reactor.callLater(
             self.RESTART_WAIT, self.start)
 
-    def _stop_vpn(self, shutdown=False, restart=False):
+    def _stop_vpn(self, restart=False):
         """
         Stops the openvpn subprocess.
 
         Attempts to send a SIGTERM first, and after a timeout
         it sends a SIGKILL.
 
-        :param shutdown: whether this is the final shutdown
-        :type shutdown: bool
         :param restart: whether this stop is part of a hard restart.
         :type restart: bool
         """
         # TODO how to return False if this fails
         # XXX maybe return a deferred
 
-        if self._vpnproc is not None:
-            self._vpnproc.restarting = restart
+        if self._vpnproc is None:
+            self.log.debug('Tried to stop VPN but no process found')
+            return
+
+        self._vpnproc.restarting = restart
+        self.__stop_pre_down(self._vpnproc)
+        self._vpnproc.terminate_or_kill()
 
+    def __stop_pre_down(self, proc):
         try:
-            if self._vpnproc is not None:
-                self._vpnproc.preDown()
+            proc.preDown()
         except Exception as e:
             self.log.error('Error on vpn pre-down {0!r}'.format(e))
             raise
 
-        d = defer.succeed(True)
-        if IS_LINUX:
-            # TODO factor this out to a linux-only launcher mechanism.
-            # First we try to be polite and send a SIGTERM...
-            if self._vpnproc is not None:
-                self._sentterm = True
-                self._vpnproc.terminate()
-
-                # we trigger a countdown to be unpolite
-                # if strictly needed.
-                d = defer.Deferred()
-                reactor.callLater(
-                    self.TERMINATE_WAIT, self._kill_if_left_alive, d)
-        return d
-
-    def _wait_and_kill(self, deferred, tries=0):
-        """
-        Check if the process is still alive, and send a
-        SIGKILL after a waiting several times during a timeout period.
-
-        :param tries: counter of tries, used in recursion
-        :type tries: int
-        """
-        if tries < self.TERMINATE_MAXTRIES:
-            if self._vpnproc.transport.pid is None:
-                deferred.callback(True)
-                return
-            else:
-                self.log.debug('Process did not die, waiting...')
-
-            tries += 1
-            reactor.callLater(
-                self.TERMINATE_WAIT,
-                self._wait_and_kill, deferred, tries)
-            return
-
-        # after running out of patience, we try a killProcess
-        self._kill(deferred)
-
-    def _kill(self, d):
-        self.log.debug('Process did not die. Sending a SIGKILL.')
-        try:
-            if self._vpnproc is None:
-                self.log.debug("There's no vpn process running to kill.")
-            else:
-                self._vpnproc.aborted = True
-                self._vpnproc.kill()
-        except OSError:
-            self.log.error('Could not kill process!')
-        d.callback(True)
-
 
 # utils