diff --git a/IPython/FakeModule.py b/IPython/FakeModule.py index cc53d99..41029f5 100644 --- a/IPython/FakeModule.py +++ b/IPython/FakeModule.py @@ -15,6 +15,37 @@ sessions. import types +def init_fakemod_dict(fm,adict=None): + """Initialize a FakeModule instance __dict__. + + Kept as a standalone function and not a method so the FakeModule API can + remain basically empty. + + This should be considered for private IPython use, used in managing + namespaces for %run. + + Parameters + ---------- + + fm : FakeModule instance + + adict : dict, optional + """ + + dct = {} + # It seems pydoc (and perhaps others) needs any module instance to + # implement a __nonzero__ method, so we add it if missing: + dct.setdefault('__nonzero__',lambda : True) + dct.setdefault('__file__',__file__) + + if adict is not None: + dct.update(adict) + + # Hard assignment of the object's __dict__. This is nasty but deliberate. + fm.__dict__.clear() + fm.__dict__.update(dct) + + class FakeModule(types.ModuleType): """Simple class with attribute access to fake a module. @@ -29,14 +60,7 @@ class FakeModule(types.ModuleType): # tmp to force __dict__ instance creation, else self.__dict__ fails self.__iptmp = None - - # It seems pydoc (and perhaps others) needs any module instance to - # implement a __nonzero__ method, so we add it if missing: - self.__dict__.setdefault('__nonzero__',lambda : True) - self.__dict__.setdefault('__file__',__file__) - # cleanup our temp trick del self.__iptmp - - if adict is not None: - self.__dict__.update(adict) + # Now, initialize the actual data in the instance dict. + init_fakemod_dict(self,adict) diff --git a/IPython/Magic.py b/IPython/Magic.py index 1546f63..72ff8f9 100644 --- a/IPython/Magic.py +++ b/IPython/Magic.py @@ -1584,23 +1584,22 @@ Currently the magic system has the following functions:\n""" prog_ns = self.shell.user_ns __name__save = self.shell.user_ns['__name__'] prog_ns['__name__'] = '__main__' - main_mod = FakeModule(prog_ns) + + ##main_mod = FakeModule(prog_ns) + main_mod = self.shell.new_main_mod(prog_ns) + else: # Run in a fresh, empty namespace if opts.has_key('n'): name = os.path.splitext(os.path.basename(filename))[0] else: name = '__main__' - main_mod = FakeModule() + + main_mod = self.shell.new_main_mod() + prog_ns = main_mod.__dict__ prog_ns['__name__'] = name - # 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). 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 @@ -1703,9 +1702,14 @@ Currently the magic system has the following functions:\n""" else: # regular execution runner(filename,prog_ns,prog_ns,exit_ignore=exit_ignore) + if opts.has_key('i'): self.shell.user_ns['__name__'] = __name__save else: + # The shell MUST hold a reference to prog_ns so after %run + # exits, the python deletion mechanism doesn't zero it out + # (leaving dangling references). + self.shell.cache_main_mod(prog_ns,filename) # update IPython interactive namespace del prog_ns['__name__'] self.shell.user_ns.update(prog_ns) @@ -1719,6 +1723,7 @@ Currently the magic system has the following functions:\n""" # added. Otherwise it will trap references to objects # contained therein. del sys.modules[main_mod_name] + self.shell.reloadhist() return stats diff --git a/IPython/iplib.py b/IPython/iplib.py index b9d758e..d674668 100644 --- a/IPython/iplib.py +++ b/IPython/iplib.py @@ -54,7 +54,7 @@ from pprint import pprint, pformat from IPython import Debugger,OInspect,PyColorize,ultraTB from IPython.ColorANSI import ColorScheme,ColorSchemeTable # too long names from IPython.Extensions import pickleshare -from IPython.FakeModule import FakeModule +from IPython.FakeModule import FakeModule, init_fakemod_dict from IPython.Itpl import Itpl,itpl,printpl,ItplNS,itplns from IPython.Logger import Logger from IPython.Magic import Magic @@ -499,13 +499,24 @@ class InteractiveShell(object,Magic): # 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 = {} + # 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. Note, + # however, that we must cache the module *namespace contents* (their + # __dict__). Because if we try to cache the actual modules, old ones + # (uncached) could be destroyed while still holding references (such as + # those held by GUI objects that tend to be long-lived)> + # + # The %reset command will flush this cache. See the cache_main_mod() + # and clear_main_mod_cache() methods for details on use. + + # This is the cache used for 'main' namespaces + self._main_ns_cache = {} + # And this is the single instance of FakeModule whose __dict__ we keep + # copying and clearing for reuse on each %run + self._user_main_module = FakeModule() # A table holding all the namespaces IPython deals with, so that # introspection facilities can search easily. @@ -521,7 +532,7 @@ class InteractiveShell(object,Magic): # a simple list. self.ns_refs_table = [ user_ns, user_global_ns, self.user_config_ns, self.alias_table, self.internal_ns, - self._user_main_modules ] + self._main_ns_cache ] # We need to insert into sys.modules something that looks like a # module but which accesses the IPython namespace, for shelve and @@ -1487,38 +1498,53 @@ class InteractiveShell(object,Magic): return True return ask_yes_no(prompt,default) - def cache_main_mod(self,mod,fname=None): - """Cache a main module. + def new_main_mod(self,ns=None): + """Return a new 'main' module object for user code execution. + """ + main_mod = self._user_main_module + init_fakemod_dict(main_mod,ns) + return main_mod + + def cache_main_mod(self,ns,fname): + """Cache a main module's namespace. - 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. + When scripts are executed via %run, we must keep a reference to the + namespace of 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. + keep one copy of the namespace (the last one), thus preventing memory + leaks from old references while allowing the objects from the last + execution to be accessible. + + Note: we can not allow the actual FakeModule instances to be deleted, + because of how Python tears down modules (it hard-sets all their + references to None without regard for reference counts). This method + must therefore make a *copy* of the given namespace, to allow the + original module's __dict__ to be cleared and reused. + Parameters ---------- - mod : a module object + ns : a namespace (a dict, typically) + + fname : str + Filename associated with the namespace. Examples -------- In [10]: import IPython - In [11]: _ip.IP.cache_main_mod(IPython) + In [11]: _ip.IP.cache_main_mod(IPython.__dict__,IPython.__file__) - In [12]: IPython.__file__ in _ip.IP._user_main_modules + In [12]: IPython.__file__ in _ip.IP._main_ns_cache Out[12]: True """ - if fname is None: - fname = mod.__file__ - #print >> sys.stderr, 'CFNAME :', os.path.abspath(fname) # dbg - self._user_main_modules[os.path.abspath(fname)] = mod + self._main_ns_cache[os.path.abspath(fname)] = ns.copy() def clear_main_mod_cache(self): """Clear the cache of main modules. @@ -1530,17 +1556,17 @@ class InteractiveShell(object,Magic): In [15]: import IPython - In [16]: _ip.IP.cache_main_mod(IPython) + In [16]: _ip.IP.cache_main_mod(IPython.__dict__,IPython.__file__) - In [17]: len(_ip.IP._user_main_modules) > 0 + In [17]: len(_ip.IP._main_ns_cache) > 0 Out[17]: True In [18]: _ip.IP.clear_main_mod_cache() - In [19]: len(_ip.IP._user_main_modules) == 0 + In [19]: len(_ip.IP._main_ns_cache) == 0 Out[19]: True """ - self._user_main_modules.clear() + self._main_ns_cache.clear() def _should_recompile(self,e): """Utility routine for edit_syntax_error""" diff --git a/IPython/tests/refbug.py b/IPython/tests/refbug.py new file mode 100644 index 0000000..66bc907 --- /dev/null +++ b/IPython/tests/refbug.py @@ -0,0 +1,38 @@ +"""Minimal script to reproduce our nasty reference counting bug. + +The problem is related to https://bugs.launchpad.net/ipython/+bug/269966 + +The original fix for that appeared to work, but JD Hunter found a matplotlib +example which, when run twice in a row, would break. The problem were +references held by open figures to internals of Tkinter. + +This code reproduces the problem that John saw, without matplotlib. We can +thus use it for our test suite. +""" + +#----------------------------------------------------------------------------- +# Module imports +#----------------------------------------------------------------------------- +import sys + +from IPython import ipapi + +#----------------------------------------------------------------------------- +# Globals +#----------------------------------------------------------------------------- +ip = ipapi.get() + +if not '_refbug_cache' in ip.user_ns: + ip.user_ns['_refbug_cache'] = [] + + +aglobal = 'Hello' +def f(): + return aglobal + +cache = ip.user_ns['_refbug_cache'] +cache.append(f) + +def call_f(): + for func in cache: + print 'lowercased:',func().lower() diff --git a/IPython/tests/test_fakemodule.py b/IPython/tests/test_fakemodule.py new file mode 100644 index 0000000..6325439 --- /dev/null +++ b/IPython/tests/test_fakemodule.py @@ -0,0 +1,17 @@ +"""Tests for the FakeModule objects. +""" + +import nose.tools as nt + +from IPython.FakeModule import FakeModule, init_fakemod_dict + +# Make a fakemod and check a few properties +def test_mk_fakemod(): + fm = FakeModule() + yield nt.assert_true,fm + yield nt.assert_true,lambda : hasattr(fm,'__file__') + +def test_mk_fakemod_fromdict(): + """Test making a FakeModule object with initial data""" + fm = FakeModule(dict(hello=True)) + nt.assert_true(fm.hello) diff --git a/IPython/tests/test_magic.py b/IPython/tests/test_magic.py index cde72f4..da5a498 100644 --- a/IPython/tests/test_magic.py +++ b/IPython/tests/test_magic.py @@ -136,6 +136,8 @@ def doctest_refbug(): """Very nasty problem with references held by multiple runs of a script. See: https://bugs.launchpad.net/ipython/+bug/269966 + In [1]: _ip.IP.clear_main_mod_cache() + In [2]: run refbug In [3]: call_f()