From 10cf84e5f8be978574b7a7e1a145903b37801753 Mon Sep 17 00:00:00 2001 From: Ivan Alejandro Date: Fri, 16 May 2014 16:17:14 -0300 Subject: Move waiting logic for imap stop to the backend. Also, improve quit and cleanup calls. --- src/leap/bitmask/backend.py | 40 +++++++---- src/leap/bitmask/gui/mainwindow.py | 107 +++++++++++++--------------- src/leap/bitmask/services/mail/conductor.py | 15 ++-- 3 files changed, 87 insertions(+), 75 deletions(-) diff --git a/src/leap/bitmask/backend.py b/src/leap/bitmask/backend.py index 49a5ca0f..9661bda0 100644 --- a/src/leap/bitmask/backend.py +++ b/src/leap/bitmask/backend.py @@ -24,6 +24,7 @@ import time from functools import partial from Queue import Queue, Empty +from threading import Condition from twisted.internet import reactor from twisted.internet import threads, defer @@ -698,6 +699,8 @@ class Mail(object): """ Interfaces with setup and launch of Mail. """ + # We give each service some time to come to a halt before forcing quit + SERVICE_STOP_TIMEOUT = 20 zope.interface.implements(ILEAPComponent) @@ -764,19 +767,25 @@ class Mail(object): """ return threads.deferToThread(self._smtp_bootstrapper.stop_smtp_service) - def stop_imap_service(self, cv): + def _stop_imap_service(self): """ - Stop imap service (fetcher, factory and port). + Stop imap and wait until the service is stopped to signal that is done. + """ + cv = Condition() + cv.acquire() + threads.deferToThread(self._imap_controller.stop_imap_service, cv) + logger.debug('Waiting for imap service to stop.') + cv.wait(self.SERVICE_STOP_TIMEOUT) + self._signaler.signal(self._signaler.IMAP_STOPPED) - :param cv: A condition variable to which we can signal when imap - indeed stops. - :type cv: threading.Condition + def stop_imap_service(self): + """ + Stop imap service (fetcher, factory and port). :returns: a defer to interact with. :rtype: twisted.internet.defer.Deferred """ - return threads.deferToThread( - self._imap_controller.stop_imap_service, cv) + return threads.deferToThread(self._stop_imap_service) class Authenticate(object): @@ -796,6 +805,7 @@ class Authenticate(object): """ self.key = "authenticate" self._signaler = signaler + self._login_defer = None self._srp_auth = SRPAuth(ProviderConfig(), self._signaler) def login(self, domain, username, password): @@ -982,6 +992,9 @@ class Signaler(QtCore.QObject): soledad_password_change_ok = QtCore.Signal(object) soledad_password_change_error = QtCore.Signal(object) + # mail related signals + imap_stopped = QtCore.Signal(object) + # This signal is used to warn the backend user that is doing something # wrong backend_bad_call = QtCore.Signal(object) @@ -1062,6 +1075,8 @@ class Signaler(QtCore.QObject): SOLEDAD_CANCELLED_BOOTSTRAP = "soledad_cancelled_bootstrap" + IMAP_STOPPED = "imap_stopped" + BACKEND_BAD_CALL = "backend_bad_call" def __init__(self): @@ -1143,6 +1158,8 @@ class Signaler(QtCore.QObject): self.SOLEDAD_PASSWORD_CHANGE_OK, self.SOLEDAD_PASSWORD_CHANGE_ERROR, + self.IMAP_STOPPED, + self.BACKEND_BAD_CALL, ] @@ -1652,15 +1669,14 @@ class Backend(object): """ self._call_queue.put(("mail", "stop_smtp_service", None)) - def stop_imap_service(self, cv): + def stop_imap_service(self): """ Stop imap service. - :param cv: A condition variable to which we can signal when imap - indeed stops. - :type cv: threading.Condition + Signals: + imap_stopped """ - self._call_queue.put(("mail", "stop_imap_service", None, cv)) + self._call_queue.put(("mail", "stop_imap_service", None)) ########################################################################### # XXX HACK: this section is meant to be a place to hold methods and diff --git a/src/leap/bitmask/gui/mainwindow.py b/src/leap/bitmask/gui/mainwindow.py index e1527bbe..557f3828 100644 --- a/src/leap/bitmask/gui/mainwindow.py +++ b/src/leap/bitmask/gui/mainwindow.py @@ -21,7 +21,6 @@ import logging import socket import time -from threading import Condition from datetime import datetime from PySide import QtCore, QtGui @@ -93,10 +92,10 @@ class MainWindow(QtGui.QMainWindow): user_stopped_eip = False # We give EIP some time to come up before starting soledad anyway - EIP_TIMEOUT = 60000 # in milliseconds + EIP_START_TIMEOUT = 60000 # in milliseconds - # We give each service some time to come to a halt before forcing quit - SERVICE_STOP_TIMEOUT = 20 + # We give the services some time to a halt before forcing quit. + SERVICES_STOP_TIMEOUT = 20 def __init__(self, quit_callback, bypass_checks=False, start_hidden=False): """ @@ -262,9 +261,6 @@ class MainWindow(QtGui.QMainWindow): # XXX should connect to mail_conductor.start_mail_service instead self.soledad_ready.connect(self._start_smtp_bootstrapping) self.soledad_ready.connect(self._start_imap_service) - self.logout.connect(self._stop_imap_service) - self.logout.connect(self._stop_smtp_service) - ################################# end Qt Signals connection ######## init_platform() @@ -282,6 +278,8 @@ class MainWindow(QtGui.QMainWindow): self._mail_conductor = mail_conductor.MailConductor(self._backend) self._mail_conductor.connect_mail_signals(self._mail_status) + self.logout.connect(self._mail_conductor.stop_mail_services) + # Eip machine is a public attribute where the state machine for # the eip connection will be available to the different components. # Remember that this will not live in the +1600LOC mainwindow for @@ -1441,17 +1439,6 @@ class MainWindow(QtGui.QMainWindow): if self._provides_mx_and_enabled(): self._mail_conductor.start_smtp_service(download_if_needed=True) - # XXX --- should remove from here, and connecte directly to the state - # machine. - @QtCore.Slot() - def _stop_smtp_service(self): - """ - TRIGGERS: - self.logout - """ - # TODO call stop_mail_service - self._mail_conductor.stop_smtp_service() - ################################################################### # Service control methods: imap @@ -1478,20 +1465,6 @@ class MainWindow(QtGui.QMainWindow): if self._provides_mx_and_enabled(): start_fun() - @QtCore.Slot() - def _stop_imap_service(self): - """ - TRIGGERS: - self.logout - """ - cv = Condition() - cv.acquire() - # TODO call stop_mail_service - threads.deferToThread(self._mail_conductor.stop_imap_service, cv) - # and wait for it to be stopped - logger.debug('Waiting for imap service to stop.') - cv.wait(self.SERVICE_STOP_TIMEOUT) - # end service control methods (imap) ################################################################### @@ -1840,7 +1813,7 @@ class MainWindow(QtGui.QMainWindow): # we want to start soledad anyway after a certain timeout if eip # fails to come up QtCore.QTimer.singleShot( - self.EIP_TIMEOUT, + self.EIP_START_TIMEOUT, self._maybe_run_soledad_setup_checks) else: if not self._already_started_eip: @@ -2015,25 +1988,20 @@ class MainWindow(QtGui.QMainWindow): # cleanup and quit methods # - def _cleanup_pidfiles(self): + def _stop_services(self): """ - Removes lockfiles on a clean shutdown. - - Triggered after aboutToQuit signal. + Stop services and cancel ongoing actions (if any). """ - if IS_WIN: - WindowsLock.release_all_locks() + logger.debug('About to quit, doing cleanup.') - def _cleanup_and_quit(self): - """ - Call all the cleanup actions in a serialized way. - Should be called from the quit function. - """ - logger.debug('About to quit, doing cleanup...') + self._cancel_ongoing_defers() - self._stop_imap_service() + logger.debug('Stopping mail services') + self._backend.stop_imap_service() + self._backend.stop_smtp_service() if self._logged_user is not None: + logger.debug("Doing logout") self._backend.logout() logger.debug('Terminating vpn') @@ -2042,7 +2010,8 @@ class MainWindow(QtGui.QMainWindow): # We need to give some time to the ongoing signals for shutdown # to come into action. This needs to be solved using # back-communication from backend. - QtCore.QTimer.singleShot(3000, self._shutdown) + # TODO: handle this, I commented this fix to merge 'cleanly' + # QtCore.QTimer.singleShot(3000, self._shutdown) def _shutdown(self): """ @@ -2061,7 +2030,8 @@ class MainWindow(QtGui.QMainWindow): def quit(self): """ - Cleanup and tidely close the main window before quitting. + Start the quit sequence and wait for services to finish. + Cleanup and close the main window before quitting. """ # TODO separate the shutting down of services from the # UI stuff. @@ -2074,22 +2044,45 @@ class MainWindow(QtGui.QMainWindow): self.tr('The app is quitting, please wait.')) # explicitly process events to display tooltip immediately - QtCore.QCoreApplication.processEvents() + QtCore.QCoreApplication.processEvents(0, 10) + + # Close other windows if any. + if self._wizard: + self._wizard.close() + + if self._logger_window: + self._logger_window.close() # Set this in case that the app is hidden QtGui.QApplication.setQuitOnLastWindowClosed(True) - self._cleanup_and_quit() + self._stop_services() - # We queue the call to stop since we need to wait until EIP is stopped. - # Otherwise we may exit leaving an unmanaged openvpn process. - reactor.callLater(0, self._backend.stop) self._really_quit = True - if self._wizard: - self._wizard.close() + # call final_quit when imap is stopped + self._backend.signaler.imap_stopped.connect(self.final_quit) + # or if we reach the timeout + reactor.callLater(self.SERVICES_STOP_TIMEOUT, self._backend.stop) - if self._logger_window: - self._logger_window.close() + @QtCore.Slot() + def final_quit(self): + """ + Final steps to quit the app, starting from here we don't care about + running services or user interaction, just quitting. + """ + logger.debug('Final quit...') + + try: + # disconnect signal if we get here due a timeout. + self._backend.signaler.imap_stopped.disconnect(self.final_quit) + except RuntimeError: + pass # Signal was not connected + + # Remove lockfiles on a clean shutdown. + logger.debug('Cleaning pidfiles') + if IS_WIN: + WindowsLock.release_all_locks() + self._backend.stop() self.close() diff --git a/src/leap/bitmask/services/mail/conductor.py b/src/leap/bitmask/services/mail/conductor.py index 67bc007e..87c4621e 100644 --- a/src/leap/bitmask/services/mail/conductor.py +++ b/src/leap/bitmask/services/mail/conductor.py @@ -66,18 +66,14 @@ class IMAPControl(object): """ self._backend.start_imap_service(self.userid, flags.OFFLINE) - def stop_imap_service(self, cv): + def stop_imap_service(self): """ Stop imap service. - - :param cv: A condition variable to which we can signal when imap - indeed stops. - :type cv: threading.Condition """ self.imap_connection.qtsigs.disconnecting_signal.emit() logger.debug('Stopping imap service.') - self._backend.stop_imap_service(cv) + self._backend.stop_imap_service() def _handle_imap_events(self, req): """ @@ -249,6 +245,13 @@ class MailConductor(IMAPControl, SMTPControl): self._smtp_machine = smtp self._smtp_machine.start() + def stop_mail_services(self): + """ + Stop the IMAP and SMTP services. + """ + self.stop_imap_service() + self.stop_smtp_service() + def connect_mail_signals(self, widget): """ Connects the mail signals to the mail_status widget slots. -- cgit v1.2.3