New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import Race Condition #108

Closed
denik opened this Issue Sep 13, 2012 · 1 comment

Comments

Projects
None yet
2 participants
@denik
Member

denik commented Sep 13, 2012

If you have two greenlets that import the same module, for example 'foo', and foo.py does something that blocks, the import will fail in one of the greenlets with 'ImportError: cannot import name foo'. This problem only occurs under gevent.

In CPython, there is some C code that handles locking around imports, ensuring that only one import is happening concurrently. This works fine for threads but does not handle greenlet switches. As far as the import lock is concerned, the two greenlets are the same thread, so it happily lets both imports go ahead, resulting in the error seen above.

I tried fixing this by monkey patching __import__ to use a gevent.coros.RLock, which successfully serialized the imports, but led to a different problem (some packages save __import__ as a class method; since it's a builtin, python knows not to pass 'self' to invocations of that method, however, a monkey-patched replacement of that builtin does not receive the same treatment. Passing _args and *_kwargs to the original import doesn't work in all situations due to this).

What steps will reproduce the problem?
See the attached tarball

What is the expected output? What do you see instead?
Should run without error, instead it throws the ImportError exception

What version of the gevent are you using?
1.0a3

On what operating system?
Happens on both CentOS and OSX

On what Python?
2.43 and 2.61

Reported by shaun@meebo-inc.com.

earlier comments
Denis.Bilenko said, at 2012-01-03T09:04:36.000Z:

Both gevent and threading scripts fail for me:

gevent-import$ python test_thread.py 
Exception in thread Thread-1:
Traceback (most recent call last):
  File "/usr/lib/python2.7/threading.py", line 552, in __bootstrap_inner
    self.run()
  File "/usr/lib/python2.7/threading.py", line 505, in run
    self.__target(*self.__args, **self.__kwargs)
  File "test_thread.py", line 6, in some_func
    from foo import x
ImportError: cannot import name x

Exception in thread Thread-2:
Traceback (most recent call last):
  File "/usr/lib/python2.7/threading.py", line 552, in __bootstrap_inner
    self.run()
  File "/usr/lib/python2.7/threading.py", line 505, in run
    self.__target(*self.__args, **self.__kwargs)
  File "test_thread.py", line 6, in some_func
    from foo import x
ImportError: cannot import name x

Should test_thread.py in the attached tar.gz run without exceptions?

shaun@meebo-inc.com said, at 2012-01-03T22:57:55.000Z:

When I was cleaning up the example before submitting, I removed the "x = 'foo'" line from foo.py, causing the "from foo import x" line to fail for obvious reasons. Bad form on my part for not re-checking it after "cleanup". Sorry about that -- here's a tarball with the correct foo.py and a patch that monkey patches __import__.

Denis.Bilenko said, at 2012-01-06T10:55:34.000Z:
Interesting patch! Are there any side-effects of patching import like that?

shaun@meebo-inc.com said, at 2012-01-07T22:18:55.000Z:

None that I've seen thus far. We've been running a version of gevent with this patch in place for a couple of weeks without any issues. I'm not particularly happy about the check for string and unicode types, but I couldn't think of a cleaner way to handle the issue. If someone is using a custom importer and bypassing __import__ entirely, this problem would still exist, but I don't know how common that is. I think custom import hooks will be protected, but I haven't tested it.

If there's a better way to monkey-patch builtin functions that handles the bound method case properly, we should use that instead, but my searches turned up nothing thus far.

As for the using the RLock around the import, that should be perfectly safe. CPython does something equivalent to protect the threaded case -- this effectively just extends that protection to greenlets as well.

schmir said, at 2012-01-17T22:17:28.000Z:
This change looks like it will break when multiple threads are involved since import_lock cannot be shared between them. You may have imp.acquire_lock() and somehow setup a thread local import_lock.

@sylvinus

This comment has been minimized.

Show comment
Hide comment

sylvinus commented Nov 23, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment