osx fixed already running instance check

7 files changed, 69 insertions, 30 deletions

diff --git a/src/leap/app.py b/src/leap/app.py
index 5f4fd656..912e390d 100644
--- a/src/leap/app.py
+++ b/src/leap/app.py
@@ -89,6 +89,10 @@ def main():
     app.setApplicationName("leap")
     app.setOrganizationDomain("leap.se")
 
+    # XXX we could check here
+    # if leap-client is already running, and abort
+    # gracefully in that case.
+
     if not QSystemTrayIcon.isSystemTrayAvailable():
         QMessageBox.critical(None, "Systray",
                              "I couldn't detect"
diff --git a/src/leap/base/constants.py b/src/leap/base/constants.py
index b38723be..f5665e5f 100644
--- a/src/leap/base/constants.py
+++ b/src/leap/base/constants.py
@@ -1,6 +1,7 @@
 """constants to be used in base module"""
 from leap import __branding
-APP_NAME = __branding.get("short_name", "leap")
+APP_NAME = __branding.get("short_name", "leap-client")
+OPENVPN_BIN = "openvpn"
 
 # default provider placeholder
 # using `example.org` we make sure that this
diff --git a/src/leap/baseapp/dialogs.py b/src/leap/baseapp/dialogs.py
index 3cb539cf..d256fc99 100644
--- a/src/leap/baseapp/dialogs.py
+++ b/src/leap/baseapp/dialogs.py
@@ -23,7 +23,8 @@ class ErrorDialog(QDialog):
 
     def warningMessage(self, msg, label):
         msgBox = QMessageBox(QMessageBox.Warning,
-                             "QMessageBox.warning()", msg,
+                             "LEAP Client Error",
+                             msg,
                              QMessageBox.NoButton, self)
         msgBox.addButton("&Ok", QMessageBox.AcceptRole)
         if msgBox.exec_() == QMessageBox.AcceptRole:
@@ -34,7 +35,8 @@ class ErrorDialog(QDialog):
 
     def criticalMessage(self, msg, label):
         msgBox = QMessageBox(QMessageBox.Critical,
-                             "QMessageBox.critical()", msg,
+                             "LEAP Client Error",
+                             msg,
                              QMessageBox.NoButton, self)
         msgBox.addButton("&Ok", QMessageBox.AcceptRole)
         msgBox.exec_()
@@ -49,7 +51,8 @@ class ErrorDialog(QDialog):
 
     def confirmMessage(self, msg, label, action):
         msgBox = QMessageBox(QMessageBox.Critical,
-                             "QMessageBox.critical()", msg,
+                             self.tr("LEAP Client Error"),
+                             msg,
                              QMessageBox.NoButton, self)
         msgBox.addButton("&Ok", QMessageBox.AcceptRole)
         msgBox.addButton("&Cancel", QMessageBox.RejectRole)
diff --git a/src/leap/baseapp/eip.py b/src/leap/baseapp/eip.py
index 4fcbee3f..03a1d6c7 100644
--- a/src/leap/baseapp/eip.py
+++ b/src/leap/baseapp/eip.py
@@ -22,6 +22,7 @@ class EIPConductorAppMixin(object):
     Connects the eip connect/disconnect logic
     to the switches in the app (buttons/menu items).
     """
+    ERR_DIALOG = False
 
     def __init__(self, *args, **kwargs):
         opts = kwargs.pop('opts')
@@ -94,6 +95,15 @@ class EIPConductorAppMixin(object):
         in the future we plan to derive errors to
         our log viewer.
         """
+        if self.ERR_DIALOG:
+            logger.warning('another error dialog suppressed')
+            return
+
+        # XXX this is actually a one-shot.
+        # On the dialog there should be 
+        # a reset signal binded to the ok button
+        # or something like that.
+        self.ERR_DIALOG = True
 
         if getattr(error, 'usermessage', None):
             message = error.usermessage
@@ -105,6 +115,7 @@ class EIPConductorAppMixin(object):
         # launching dialog.
         # (so Qt tests can assert stuff)
 
+
         if error.critical:
             logger.critical(error.message)
             #critical error (non recoverable),
@@ -113,6 +124,7 @@ class EIPConductorAppMixin(object):
             ErrorDialog(errtype="critical",
                         msg=message,
                         label="critical error")
+
         elif error.warning:
             logger.warning(error.message)
 
diff --git a/src/leap/eip/exceptions.py b/src/leap/eip/exceptions.py
index c127a58f..b7d398c3 100644
--- a/src/leap/eip/exceptions.py
+++ b/src/leap/eip/exceptions.py
@@ -106,11 +106,11 @@ class LeapBadConfigFetchedError(Warning):
         "an error occurred during configuratio of leap services")
 
 
-class OpenVPNAlreadyRunning(EIPClientError):
+class OpenVPNAlreadyRunning(CriticalError):
     message = "Another OpenVPN Process is already running."
     usermessage = translate(
         "EIPErrors",
-        "Another OpenVPN Process has been detected."
+        "Another OpenVPN Process has been detected. "
         "Please close it before starting leap-client")
 
 
diff --git a/src/leap/eip/openvpnconnection.py b/src/leap/eip/openvpnconnection.py
index e5169465..05979ff7 100644
--- a/src/leap/eip/openvpnconnection.py
+++ b/src/leap/eip/openvpnconnection.py
@@ -2,18 +2,21 @@
 OpenVPN Connection
 """
 from __future__ import (print_function)
+from functools import partial
 import logging
 import os
 import psutil
 import shutil
 import select
 import socket
-from functools import partial
+from time import sleep
 
 logger = logging.getLogger(name=__name__)
 
 from leap.base.connection import Connection
+from leap.base.constants import OPENVPN_BIN
 from leap.util.coroutines import spawn_and_watch_process
+from leap.util.misc import get_openvpn_pids
 
 from leap.eip.udstelnet import UDSTelnet
 from leap.eip import config as eip_config
@@ -277,23 +280,20 @@ to be triggered for each one of them.
 
     # checks
 
+
     def _check_if_running_instance(self):
         """
         check if openvpn is already running
         """
-        try:
-            #FIXME this gives DeprecationWarning
-            for process in psutil.get_process_list():
-                if process.name == "openvpn":
-                    logger.debug('an openvpn instance is already running.')
-                    logger.debug('attempting to stop openvpn instance.')
-                    if not self._stop_openvpn():
-                        raise eip_exceptions.OpenVPNAlreadyRunning
-
-        except psutil.error.NoSuchProcess:
-            logger.debug('detected a process which died. passing.')
-
-        logger.debug('no openvpn instance found.')
+        openvpn_pids = get_openvpn_pids()
+        if openvpn_pids:
+            logger.debug('an openvpn instance is already running.')
+            logger.debug('attempting to stop openvpn instance.')
+            if not self._stop_openvpn():
+                raise eip_exceptions.OpenVPNAlreadyRunning
+            return
+        else:
+            logger.debug('no openvpn instance found.')
 
     def _set_ovpn_command(self):
         try:
@@ -334,7 +334,7 @@ to be triggered for each one of them.
         #deprecate watcher_cb,
         #use _only_ signal_maps instead
 
-        logger.debug('_launch_openvpn called')
+        #logger.debug('_launch_openvpn called')
         if self.watcher_cb is not None:
             linewrite_callback = self.watcher_cb
         else:
@@ -364,23 +364,24 @@ to be triggered for each one of them.
         interface
         """
         # XXX method a bit too long, split
-        logger.debug("terminating openvpn process...")
+        logger.debug("atempting to terminate openvpn process...")
         if self.connected():
             try:
                 self._send_command("signal SIGTERM\n")
+                sleep(1)
+                if not self.subp:  # XXX ???
+                    return True
             except socket.error:
                 logger.warning('management socket died')
                 return
 
-        if self.subp:
-            # ???
-            return True
 
         #shutting openvpn failured
         #try patching in old openvpn host and trying again
+        # XXX could be more than one!
         process = self._get_openvpn_process()
         if process:
-            logger.debug('process :%s' % process)
+            logger.debug('process: %s' % process.name)
             cmdline = process.cmdline
 
             manag_flag = "--management"
@@ -401,10 +402,8 @@ to be triggered for each one of them.
         return True
 
     def _get_openvpn_process(self):
-        # plist = [p for p in psutil.get_process_list() if p.name == "openvpn"]
-        # return plist[0] if plist else None
-        for process in psutil.get_process_list():
-            if process.name == "openvpn":
+        for process in psutil.process_iter():
+            if OPENVPN_BIN in process.name:
                 return process
         return None
 
diff --git a/src/leap/util/misc.py b/src/leap/util/misc.py
index 3c26892b..aa3ebe25 100644
--- a/src/leap/util/misc.py
+++ b/src/leap/util/misc.py
@@ -1,6 +1,9 @@
 """
 misc utils
 """
+import psutil
+
+from leap.base.constants import OPENVPN_BIN
 
 
 class ImproperlyConfigured(Exception):
@@ -14,3 +17,20 @@ def null_check(value, value_name):
     except AssertionError:
         raise ImproperlyConfigured(
             "%s parameter cannot be None" % value_name)
+        
+def get_openvpn_pids():
+    # binary name might change
+
+    openvpn_pids = []
+    for p in psutil.process_iter():
+        try:
+            # XXX Not exact!
+            # Will give false positives.
+            # we should check that cmdline BEGINS
+            # with openvpn or with our wrapper
+            # (pkexec / osascript / whatever)
+            if OPENVPN_BIN in ' '.join(p.cmdline):
+                openvpn_pids.append(p.pid)
+        except psutil.error.AccessDenied:
+            pass
+    return openvpn_pids