From 2f5ba0909e1a28ab7e734d56a9a584d1430d4426 2009-03-13 17:56:19 From: Fernando Perez Date: 2009-03-13 17:56:19 Subject: [PATCH] Fix bug: https://bugs.launchpad.net/ipython/+bug/269966 This fixes a long-standing and serious problem with long-running IPython sessions. Finally! Many thanks to John D. Hunter and Sameer D'Costa for the feedback, especially for Sameer's patch that finally put me on the right track for a clean solution. --- diff --git a/IPython/Magic.py b/IPython/Magic.py index aa188b6..e9d3437 100644 --- a/IPython/Magic.py +++ b/IPython/Magic.py @@ -13,11 +13,6 @@ #**************************************************************************** # Modules and globals -from IPython import Release -__author__ = '%s <%s>\n%s <%s>' % \ - ( Release.authors['Janko'] + Release.authors['Fernando'] ) -__license__ = Release.license - # Python standard modules import __builtin__ import bdb @@ -1049,10 +1044,33 @@ Currently the magic system has the following functions:\n""" def magic_reset(self, parameter_s=''): """Resets the namespace by removing all names defined by the user. - Input/Output history are left around in case you need them.""" + Input/Output history are left around in case you need them. + + Parameters + ---------- + -y : force reset without asking for confirmation. + + Examples + -------- + In [6]: a = 1 + + In [7]: a + Out[7]: 1 + + In [8]: 'a' in _ip.user_ns + Out[8]: True + + In [9]: %reset -f - ans = self.shell.ask_yes_no( - "Once deleted, variables cannot be recovered. Proceed (y/[n])? ") + In [10]: 'a' in _ip.user_ns + Out[10]: False + """ + + if parameter_s == '-f': + ans = True + else: + ans = self.shell.ask_yes_no( + "Once deleted, variables cannot be recovered. Proceed (y/[n])? ") if not ans: print 'Nothing done.' return @@ -1062,7 +1080,7 @@ Currently the magic system has the following functions:\n""" # Also flush the private list of module references kept for script # execution protection - self.shell._user_main_modules[:] = [] + self.shell.clear_main_mod_cache() def magic_logstart(self,parameter_s=''): """Start logging anywhere in a session. @@ -1576,25 +1594,10 @@ Currently the magic system has the following functions:\n""" # The shell MUST hold a reference to main_mod so after %run exits, # the python deletion mechanism doesn't zero it out (leaving - # dangling references) - - # XXX - the note above was written without further detail, but this - # code actually causes problems. By holding references to the - # namespace where every script is executed, we effectively disable - # just about all possible variable cleanup. In particular, - # generator expressions and other variables that point to open - # files are kept alive, and as a user session lives on, it may run - # out of available file descriptors. Such a bug has already been - # reported by JD Hunter. I'm disabling this for now, but we need - # to clarify exactly (and add tests) what from main_mod needs to be - # kept alive and what is save to remove... In fact, see note - # below, where we append main_mod to sys.modules and then delete it - # again. The final cleanup is rendered moot by this reference kept - # in _user_main_modules(), so we really need to look into this. - - self.shell._user_main_modules.append(main_mod) - - # /XXX + # dangling references). However, we should drop old versions of + # main_mod. There is now a proper API to manage this caching in + # the main shell object, we use that. + self.shell.cache_main_mod(main_mod) # Since '%run foo' emulates 'python foo.py' at the cmd line, we must # set the __file__ global in the script's namespace diff --git a/IPython/iplib.py b/IPython/iplib.py index cf05a76..e54fe24 100644 --- a/IPython/iplib.py +++ b/IPython/iplib.py @@ -335,15 +335,19 @@ class InteractiveShell(object,Magic): # code ran is deleted. Now that this object is a true module (needed # so docetst and other tools work correctly), the Python module # teardown mechanism runs over it, and sets to None every variable - # present in that module. This means that later calls to functions - # defined in the script (which have become interactively visible after - # script exit) fail, because they hold references to objects that have - # become overwritten into None. The only solution I see right now is - # to protect every FakeModule used by %run by holding an internal - # reference to it. This private list will be used for that. The - # %reset command will flush it as well. - self._user_main_modules = [] - + # present in that module. Top-level references to objects from the + # script survive, because the user_ns is updated with them. However, + # calling functions defined in the script that use other things from + # the script will fail, because the function's closure had references + # to the original objects, which are now all None. So we must protect + # these modules from deletion by keeping a cache. To avoid keeping + # stale modules around (we only need the one from the last run), we use + # a dict keyed with the full path to the script, so only the last + # version of the module is held in the cache. The %reset command will + # flush this cache. See the cache_main_mod() and clear_main_mod_cache() + # methods for details on use. + self._user_main_modules = {} + # List of input with multi-line handling. # Fill its zero entry, user counter starts at 1 self.input_hist = InputList(['\n']) @@ -594,10 +598,6 @@ class InteractiveShell(object,Magic): #TODO: remove this, redundant self.add_builtins() - - - - # end __init__ def var_expand(self,cmd,depth=0): @@ -626,16 +626,15 @@ class InteractiveShell(object,Magic): """ rc = self.rc try: - self.db = pickleshare.PickleShareDB(rc.ipythondir + "/db") + self.db = pickleshare.PickleShareDB(rc.ipythondir + "/db") except exceptions.UnicodeDecodeError: print "Your ipythondir can't be decoded to unicode!" print "Please set HOME environment variable to something that" print r"only has ASCII characters, e.g. c:\home" print "Now it is",rc.ipythondir sys.exit() - self.shadowhist = IPython.history.ShadowHist(self.db) - - + self.shadowhist = IPython.history.ShadowHist(self.db) + def post_config_initialization(self): """Post configuration init method @@ -902,7 +901,6 @@ class InteractiveShell(object,Magic): call_pdb = property(_get_call_pdb,_set_call_pdb,None, 'Control auto-activation of pdb at exceptions') - # These special functions get installed in the builtin namespace, to # provide programmatic (pure python) access to magics, aliases and system # calls. This is important for logging, user scripting, and more. @@ -1132,7 +1130,8 @@ IPython will create a minimal default configuration for you. inif = 'ipythonrc.ini' else: inif = 'ipythonrc' - minimal_setup = {'ipy_user_conf.py' : 'import ipy_defaults', inif : '# intentionally left blank' } + minimal_setup = {'ipy_user_conf.py' : 'import ipy_defaults', + inif : '# intentionally left blank' } os.makedirs(ipythondir, mode = 0777) for f, cont in minimal_setup.items(): open(ipythondir + '/' + f,'w').write(cont) @@ -1291,7 +1290,6 @@ want to merge them back into the new files.""" % locals() finally: readline.read_history_file(self.histfile) return wrapper - def pre_readline(self): """readline hook to be used at the start of each line. @@ -1389,7 +1387,59 @@ want to merge them back into the new files.""" % locals() if self.rc.quiet: return True return ask_yes_no(prompt,default) - + + def cache_main_mod(self,mod): + """Cache a main module. + + When scripts are executed via %run, we must keep a reference to their + __main__ module (a FakeModule instance) around so that Python doesn't + clear it, rendering objects defined therein useless. + + This method keeps said reference in a private dict, keyed by the + absolute path of the module object (which corresponds to the script + path). This way, for multiple executions of the same script we only + keep one copy of __main__ (the last one), thus preventing memory leaks + from old references while allowing the objects from the last execution + to be accessible. + + Parameters + ---------- + mod : a module object + + Examples + -------- + + In [10]: import IPython + + In [11]: _ip.IP.cache_main_mod(IPython) + + In [12]: IPython.__file__ in _ip.IP._user_main_modules + Out[12]: True + """ + self._user_main_modules[os.path.abspath(mod.__file__) ] = mod + + def clear_main_mod_cache(self): + """Clear the cache of main modules. + + Mainly for use by utilities like %reset. + + Examples + -------- + + In [15]: import IPython + + In [16]: _ip.IP.cache_main_mod(IPython) + + In [17]: len(_ip.IP._user_main_modules) > 0 + Out[17]: True + + In [18]: _ip.IP.clear_main_mod_cache() + + In [19]: len(_ip.IP._user_main_modules) == 0 + Out[19]: True + """ + self._user_main_modules.clear() + def _should_recompile(self,e): """Utility routine for edit_syntax_error""" @@ -1545,8 +1595,6 @@ want to merge them back into the new files.""" % locals() self.set_completer() except KeyboardInterrupt: self.write("\nKeyboardInterrupt\n") - - def mainloop(self,banner=None): """Creates the local namespace and starts the mainloop. @@ -1569,7 +1617,9 @@ want to merge them back into the new files.""" % locals() try: self.interact(banner) #self.interact_with_readline() - # XXX for testing of a readline-decoupled repl loop, call interact_with_readline above + + # XXX for testing of a readline-decoupled repl loop, call + # interact_with_readline above break except KeyboardInterrupt: diff --git a/IPython/tests/tclass.py b/IPython/tests/tclass.py index 9b62fde..1e8e1dd 100644 --- a/IPython/tests/tclass.py +++ b/IPython/tests/tclass.py @@ -1,6 +1,26 @@ """Simple script to instantiate a class for testing %run""" +import sys + +# An external test will check that calls to f() work after %run class foo: pass def f(): - foo() + return foo() + +# We also want to ensure that while objects remain available for immediate +# access, objects from *previous* runs of the same script get collected, to +# avoid accumulating massive amounts of old references. +class C(object): + def __init__(self,name): + self.name = name + + def __del__(self): + print 'Deleting object:',self.name + +try: + name = sys.argv[1] +except IndexError: + pass +else: + c = C(name) diff --git a/IPython/tests/test_magic.py b/IPython/tests/test_magic.py index e2b69f8..7208416 100644 --- a/IPython/tests/test_magic.py +++ b/IPython/tests/test_magic.py @@ -37,11 +37,23 @@ def test_rehashx(): def doctest_run_ns(): """Classes declared %run scripts must be instantiable afterwards. + In [11]: run tclass + + In [12]: isinstance(f(),foo) + Out[12]: True + """ + + +def doctest_run_ns2(): + """Classes declared %run scripts must be instantiable afterwards. + In [3]: run tclass.py - In [4]: f() + In [4]: run tclass first_pass + + In [5]: run tclass second_pass + Deleting object: first_pass """ - pass # doctest only def doctest_hist_f():