From b8f41d55b3674d436d6138abae85ffecedab183b 2011-12-06 01:40:59 From: MinRK Date: 2011-12-06 01:40:59 Subject: [PATCH] improve cleanup of connection files * allow clean shutdown of qtconsole on sigint * init_kernel_manager registers cleanup_connection_file with atexit * connection_file trait is guaranteed to be abspath at end of init_connection_file(), so find methods are no longer necessary after this call. * move cf removal from KM.__del__ to KM.cleanup_connection_file * _new_connection_file only uses last segment of uuid, not the whole thing, which is ugly and overkill 2**48 is enough kernels. --- diff --git a/IPython/frontend/kernelmixinapp.py b/IPython/frontend/kernelmixinapp.py index 6483528..d7f7ab7 100644 --- a/IPython/frontend/kernelmixinapp.py +++ b/IPython/frontend/kernelmixinapp.py @@ -21,6 +21,7 @@ Authors: #----------------------------------------------------------------------------- # stdlib imports +import atexit import json import os import signal @@ -234,6 +235,9 @@ class IPythonMixinConsoleApp(Configurable): argument does not match an existing file, it will be interpreted as a fileglob, and the matching file in the current profile's security dir with the latest access time will be used. + + After this method is called, self.connection_file contains the *full path* + to the connection file, never just its name. """ if self.existing: try: @@ -243,6 +247,20 @@ class IPythonMixinConsoleApp(Configurable): self.exit(1) self.log.info("Connecting to existing kernel: %s" % cf) self.connection_file = cf + else: + # not existing, check if we are going to write the file + # and ensure that self.connection_file is a full path, not just the shortname + try: + cf = find_connection_file(self.connection_file) + except Exception: + # file might not exist + if self.connection_file == os.path.basename(self.connection_file): + # just shortname, put it in security dir + cf = os.path.join(self.profile_dir.security_dir, self.connection_file) + else: + cf = self.connection_file + self.connection_file = cf + # should load_connection_file only be used for existing? # as it is now, this allows reusing ports if an existing # file is requested @@ -315,21 +333,21 @@ class IPythonMixinConsoleApp(Configurable): self.log.critical("--existing %s" % self.connection_file) def _new_connection_file(self): - return os.path.join(self.profile_dir.security_dir, 'kernel-%s.json' % uuid.uuid4()) + cf = '' + while not cf: + # we don't need a 128b id to distinguish kernels, use more readable + # 48b node segment (12 hex chars). Users running more than 32k simultaneous + # kernels can subclass. + ident = str(uuid.uuid4()).split('-')[-1] + cf = os.path.join(self.profile_dir.security_dir, 'kernel-%s.json' % ident) + # only keep if it's actually new. Protect against unlikely collision + # in 48b random search space + cf = cf if not os.path.exists(cf) else '' + return cf def init_kernel_manager(self): # Don't let Qt or ZMQ swallow KeyboardInterupts. signal.signal(signal.SIGINT, signal.SIG_DFL) - sec = self.profile_dir.security_dir - try: - cf = filefind(self.connection_file, ['.', sec]) - except IOError: - # file might not exist - if self.connection_file == os.path.basename(self.connection_file): - # just shortname, put it in security dir - cf = os.path.join(sec, self.connection_file) - else: - cf = self.connection_file # Create a KernelManager and start a kernel. self.kernel_manager = self.kernel_manager_class( @@ -338,7 +356,7 @@ class IPythonMixinConsoleApp(Configurable): iopub_port=self.iopub_port, stdin_port=self.stdin_port, hb_port=self.hb_port, - connection_file=cf, + connection_file=self.connection_file, config=self.config, ) # start the kernel @@ -349,6 +367,7 @@ class IPythonMixinConsoleApp(Configurable): elif self.sshserver: # ssh, write new connection file self.kernel_manager.write_connection_file() + atexit.register(self.kernel_manager.cleanup_connection_file) self.kernel_manager.start_channels() diff --git a/IPython/frontend/qt/console/qtconsoleapp.py b/IPython/frontend/qt/console/qtconsoleapp.py index 5e5b72c..4e613ca 100644 --- a/IPython/frontend/qt/console/qtconsoleapp.py +++ b/IPython/frontend/qt/console/qtconsoleapp.py @@ -27,7 +27,7 @@ import sys import uuid # System library imports -from IPython.external.qt import QtGui +from IPython.external.qt import QtCore, QtGui # Local imports from IPython.config.application import boolean_flag, catch_config_error @@ -298,12 +298,26 @@ class IPythonQtConsoleApp(BaseIPythonApplication, IPythonMixinConsoleApp): else: raise IOError("Stylesheet %r not found."%self.stylesheet) + def init_signal(self): + """allow clean shutdown on sigint""" + signal.signal(signal.SIGINT, lambda sig, frame: self.exit(-2)) + # need a timer, so that QApplication doesn't block until a real + # Qt event fires (can require mouse movement) + # timer trick from http://stackoverflow.com/q/4938723/938949 + timer = QtCore.QTimer() + # Let the interpreter run each 200 ms: + timer.timeout.connect(lambda: None) + timer.start(200) + # hold onto ref, so the timer doesn't get cleaned up + self._sigint_timer = timer + @catch_config_error def initialize(self, argv=None): super(IPythonQtConsoleApp, self).initialize(argv) IPythonMixinConsoleApp.initialize(self,argv) self.init_qt_elements() self.init_colors() + self.init_signal() def start(self): diff --git a/IPython/zmq/kernelmanager.py b/IPython/zmq/kernelmanager.py index f7f856d..dc7a0fe 100644 --- a/IPython/zmq/kernelmanager.py +++ b/IPython/zmq/kernelmanager.py @@ -639,14 +639,8 @@ class KernelManager(HasTraits): self.session = Session(config=self.config) def __del__(self): - if self._connection_file_written: - # cleanup connection files on full shutdown of kernel we started - self._connection_file_written = False - try: - os.remove(self.connection_file) - except IOError: - pass - + self.cleanup_connection_file() + #-------------------------------------------------------------------------- # Channel management methods: @@ -694,6 +688,19 @@ class KernelManager(HasTraits): # Kernel process management methods: #-------------------------------------------------------------------------- + def cleanup_connection_file(self): + """cleanup connection file *if we wrote it* + + Will not raise if the connection file was already removed somehow. + """ + if self._connection_file_written: + # cleanup connection files on full shutdown of kernel we started + self._connection_file_written = False + try: + os.remove(self.connection_file) + except OSError: + pass + def load_connection_file(self): """load connection info from JSON dict in self.connection_file""" with open(self.connection_file) as f: