Opened 11 years ago

Closed 11 years ago

#3913 defect closed fixed (fixed)

Twisted plugins fail when import path has trailing slash

Reported by: Colin Alston Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/path-importer-cache-inconsistency-3913
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

The essence of the problem is if you have a trailing slash in your import path, twisted asserts that the stripped module path is in the python import cache. Obviously "'/foo/bar' in {'/foo/bar/':None}" never matches anything, then many kittens die.

import sys
sys.path.append('/usr/local/tcs/tums/lib/')

Worlds largest traceback commenses here

2009-07-13 09:22:45+0200 [HTTPChannel,1,172.31.0.127] Unhandled Error
        Traceback (most recent call last):
          File "/usr/local/tcs/tums/lib/nevow/rend.py", line 572, in _renderHTTP
            return self.flattenFactory(doc, ctx, writer, finisher)
          File "/usr/local/tcs/tums/lib/nevow/rend.py", line 531, in <lambda>
            flattenFactory = lambda self, *args: flat.flattenFactory(*args)
          File "/usr/local/tcs/tums/lib/nevow/flat/__init__.py", line 14, in flattenFactory
            return deferflatten(stan, ctx, writer).addCallback(finisher)
          File "/usr/local/tcs/tums/lib/nevow/flat/twist.py", line 63, in deferflatten
            _drive(iterable, finished)
        --- <exception caught here> ---
          File "/usr/local/tcs/tums/lib/nevow/flat/twist.py", line 24, in _drive
            next = iterable.next()
          File "/usr/local/tcs/tums/lib/nevow/flat/ten.py", line 83, in iterflatten
            for item in gen:
          File "/usr/local/tcs/tums/lib/nevow/flat/flatstan.py", line 103, in TagSerializer
            yield serialize(toBeRenderedBy, context)
          File "/usr/local/tcs/tums/lib/nevow/flat/ten.py", line 70, in serialize
            return partialflatten(context, obj)
          File "/usr/local/tcs/tums/lib/nevow/flat/ten.py", line 61, in partialflatten
            return flattener(obj, context)
          File "/usr/local/tcs/tums/lib/nevow/flat/flatstan.py", line 264, in DirectiveSerializer
            return serialize(renderer, context)
          File "/usr/local/tcs/tums/lib/nevow/flat/ten.py", line 70, in serialize
            return partialflatten(context, obj)
          File "/usr/local/tcs/tums/lib/nevow/flat/ten.py", line 61, in partialflatten
            return flattener(obj, context)
          File "/usr/local/tcs/tums/lib/nevow/flat/flatstan.py", line 247, in MethodSerializer
            return FunctionSerializer(original, context, nocontext)
          File "/usr/local/tcs/tums/lib/nevow/flat/flatstan.py", line 236, in FunctionSerializer
            result = original(context, data)
          File "/usr/local/tcs/tums/lib/nevow/athena.py", line 1064, in render_liveglue
            in self._getRequiredModules()],
          File "/usr/local/tcs/tums/lib/nevow/athena.py", line 755, in _getRequiredModules
            in self._getModuleForClass().allDependencies()
          File "/usr/local/tcs/tums/lib/nevow/athena.py", line 742, in _getModuleForClass
            return jsDeps.getModuleForClass(self.jsClass)
          File "/usr/local/tcs/tums/lib/nevow/athena.py", line 301, in getModuleForName
            self.mapping.update(allJavascriptPackages())
          File "/usr/local/tcs/tums/lib/nevow/athena.py", line 273, in allJavascriptPackages
            for p in plugin.getPlugIns(inevow.IJavascriptPackage, plugins):
          File "/usr/lib/python2.5/site-packages/twisted/plugin.py", line 200, in getPlugins
            allDropins = getCache(package)
          File "/usr/lib/python2.5/site-packages/twisted/plugin.py", line 131, in getCache
            mod = getModule(module.__name__)
          File "/usr/lib/python2.5/site-packages/twisted/python/modules.py", line 741, in getModule
            return theSystemPath[moduleName]
          File "/usr/lib/python2.5/site-packages/twisted/python/modules.py", line 677, in __getitem__
            self._findEntryPathString(moduleObject)),
          File "/usr/lib/python2.5/site-packages/twisted/python/modules.py", line 630, in _findEntryPathString
            rval, modobj, pformat(self.importerCache))
        exceptions.AssertionError: '/usr/local/tcs/tums/lib' for <module 'nevow.plugins' from '/usr/local/tcs/tums/lib/nevow/plugins/__init__.pyc'> not in import cache {'.': None,
         '/usr/lib/python2.5': None,
         '/usr/lib/python2.5/': None,
         '/usr/lib/python2.5/ctypes': None,
         '/usr/lib/python2.5/email': None,
         '/usr/lib/python2.5/email/mime': None,
         '/usr/lib/python2.5/lib-dynload': None,
         '/usr/lib/python2.5/lib-tk': None,
         '/usr/lib/python2.5/plat-linux2': None,
         '/usr/lib/python2.5/site-packages': None,
         '/usr/lib/python2.5/site-packages/Crypto': None,
         '/usr/lib/python2.5/site-packages/Crypto/Cipher': None,
         '/usr/lib/python2.5/site-packages/Crypto/Hash': None,
         '/usr/lib/python2.5/site-packages/Crypto/PublicKey': None,
         '/usr/lib/python2.5/site-packages/Crypto/Util': None,
         '/usr/lib/python2.5/site-packages/PIL': None,
         '/usr/lib/python2.5/site-packages/cairo': None,
         '/usr/lib/python2.5/site-packages/pysqlite2': None,
         '/usr/lib/python2.5/site-packages/twisted': None,
         '/usr/lib/python2.5/site-packages/twisted/application': None,
         '/usr/lib/python2.5/site-packages/twisted/conch': None,
         '/usr/lib/python2.5/site-packages/twisted/conch/insults': None,
         '/usr/lib/python2.5/site-packages/twisted/conch/ssh': None,
         '/usr/lib/python2.5/site-packages/twisted/cred': None,
         '/usr/lib/python2.5/site-packages/twisted/internet': None,
         '/usr/lib/python2.5/site-packages/twisted/mail': None,
         '/usr/lib/python2.5/site-packages/twisted/persisted': None,
         '/usr/lib/python2.5/site-packages/twisted/protocols': None,
         '/usr/lib/python2.5/site-packages/twisted/python': None,
         '/usr/lib/python2.5/site-packages/twisted/scripts': None,
         '/usr/lib/python2.5/site-packages/twisted/spread': None,
         '/usr/lib/python2.5/site-packages/twisted/trial': None,
         '/usr/lib/python2.5/site-packages/twisted/web': None,
         '/usr/lib/python2.5/site-packages/zope': None,
         '/usr/lib/python2.5/site-packages/zope/interface': None,
         '/usr/lib/python2.5/xml': None,
         '/usr/lib/python2.5/xml/parsers': None,
         '/usr/lib/python2.5/xml/sax': None,
         '/usr/local/lib/python2.5/site-packages': None,
         '/usr/local/tcs/tums': None,
         '/usr/local/tcs/tums/lang': None,
         '/usr/local/tcs/tums/lang/en': None,
         '/usr/local/tcs/tums/lib/': None,
         '/usr/local/tcs/tums/lib/axiom': None,
         '/usr/local/tcs/tums/lib/epsilon': None,
         '/usr/local/tcs/tums/lib/formal': None,
         '/usr/local/tcs/tums/lib/formal/widgets': None,
         '/usr/local/tcs/tums/lib/formless': None,
         '/usr/local/tcs/tums/lib/nevow': None,
         '/usr/local/tcs/tums/lib/nevow/flat': None,
         '/usr/local/tcs/tums/lib/nevow/plugins': None,
         '/usr/local/tcs/tums/lib/nevow/taglibrary': None,
         '/usr/local/tcs/tums/lib/pycha': None,
         '/usr/local/tcs/tums/lib/reportlab': None,
         '/usr/local/tcs/tums/lib/reportlab/lib': None,
         '/usr/local/tcs/tums/lib/reportlab/pdfbase': None,
         '/usr/local/tcs/tums/lib/reportlab/pdfgen': None,
         '/usr/local/tcs/tums/lib/reportlab/platypus': None,
         '/usr/local/tcs/tums/lib/sasync': None,
         '/usr/local/tcs/tums/lib/sqlalchemy': None,
         '/usr/local/tcs/tums/lib/sqlalchemy/databases': None,
         '/usr/local/tcs/tums/lib/sqlalchemy/engine': None,
         '/usr/local/tcs/tums/lib/sqlalchemy/orm': None,
         '/var/lib/python-support/python2.5': None,
         '/var/lib/python-support/python2.5/MySQLdb': None,
         '/var/lib/python-support/python2.5/MySQLdb/constants': None,
         '/var/lib/python-support/python2.5/OpenSSL': None}

2009-07-13 09:22:46+0200 [HTTPChannel,1,172.31.0.127] <class 'nevow.stan.raw'> ERROR
...

Change History (7)

comment:1 Changed 11 years ago by Jean-Paul Calderone

I think appending a path ending in '/' to sys.path is technically a violation of PEP 302, since it causes the invariant described in that PEP to cease to hold (as the failed assertion shows).

Since touching sys.path in application code is (unfortunately) quite common, twisted.python.modules should probably try to handle this case anyway, though.

As I implied, the exception which occurs in this case is an AssertionError. One resolution of this issue would be to simply delete the assertion. The consistency of the path importer cache is beyond the control of twisted.python.modules. It would certainly be nice if it were always consistent, but the point in the code where this assertion fails is not the point where the inconsistency is introduced. That happens elsewhere, when someone messes with sys.path (or possibly does something else). Turning this into a failure much later in code completely unrelated to the problem doesn't seem useful.

A slightly weaker change would be to turn the assertion into a check which only emits a warning about a site misconfiguration. This would still point out the problem (sort of - as well as the warning system ever can), but allow code to basically continue to work.

I can't think of any other reasonable solutions. Anything that involved canonicalizing paths is probably unreasonable, since these objects don't "belong" to twisted.python.modules. The code could canonicalize the path names before making any checks, which would probably address this problem, but at the cost of a lot of extra path manipulation for each module lookup. And still, the most invasive thing it should do should any check fail is emit a warning. Also, given that PEP 302 doesn't talk about path canonicalization being required, this would probably be a bad idea.

comment:2 Changed 11 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/path-importer-cache-inconsistency-3913

(In [27129]) Branching to 'path-importer-cache-inconsistency-3913'

comment:3 Changed 11 years ago by Jean-Paul Calderone

Keywords: review added
Owner: Glyph deleted

I implemented the warning thing.

I feel like there are a lot of holes in this code (#3674, #2507, #2489, #2005) and it might benefit from a higher-level approach. Or not.

comment:4 Changed 11 years ago by therve

Keywords: review removed
Owner: set to Jean-Paul Calderone

I like that, please update copyright notices of both files and merge.

comment:5 Changed 11 years ago by Jean-Paul Calderone

(In [27160]) Update copyright dates

refs #3913

comment:6 Changed 11 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [27161]) Merge path-importer-cache-inconsistency-3913

Author: exarkun Reviewer: therve Fixes: #3913

Change the PEP 302 path_importer_cache consistency assertion into a regular check with a warning emitted if the check fails. This prevents a common misconfiguration which is generally harmless from completely disabling module lookup via the twisted.python.modules APIs, but still lets the user know there might be something amiss.

comment:7 Changed 10 years ago by <automation>

Owner: Jean-Paul Calderone deleted
Note: See TracTickets for help on using tickets.