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

Fix monkey_patch() on Python 3 #168

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@vstinner
Contributor

vstinner commented Nov 20, 2014

The importlib module must use real thread locks, not eventlet.Semaphore.

The added unit test hangs on Python 3 without the fix.

Fix monkey_patch() on Python 3
The importlib module must use real thread locks, not eventlet.Semaphore.
if sys.version_info >= (3, 3):
import importlib._bootstrap
thread = original('_thread')
# importlib must use real thread locks, not eventlet.Semaphore

This comment has been minimized.

@temoto

temoto Nov 24, 2014

Member

Why?

This comment has been minimized.

@vstinner

vstinner Dec 11, 2014

Contributor

I added a long explanation in the pull request, did you see it?

@vstinner

vstinner Dec 11, 2014

Contributor

I added a long explanation in the pull request, did you see it?

This comment has been minimized.

@temoto

temoto Dec 12, 2014

Member

Yes, thank you, it makes sense now.

@temoto

temoto Dec 12, 2014

Member

Yes, thank you, it makes sense now.

@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Nov 27, 2014

Contributor

Why?

importlib cannot use eventlet locks because event locks need to import modules.

It looks like the first call the import in the thread calls eventlet.event.Event().acquire() which calls get_hub() which executes (indirectly) "import eventlet.hubs.epolls" which retries to acquire the same event and the loop restarts.

The eventlet.hubs.epolls module is required to setup the hub. In my test, the hub is first setup in a thread, not in the main thread. The problem is that the hub is setup to acquire a lock to import a module, but setup the hub has to import a lock, whereas the hub requires to import a second module.

I guess that calling "get_hub()" in a thread is enough to reproduce the issue.

The importlib modules maintains a dictionary of locks using the thread identifier for the key.

Traceback raises by patcher_test without the fix:

AttributeError: '_thread._local' object has no attribute 'hub'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 112, in get_hub
    _threadlocal.Hub
AttributeError: '_thread._local' object has no attribute 'Hub'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 109, in get_hub
    hub = _threadlocal.hub
AttributeError: '_thread._local' object has no attribute 'hub'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 112, in get_hub
    _threadlocal.Hub
AttributeError: '_thread._local' object has no attribute 'Hub'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 109, in get_hub
    hub = _threadlocal.hub
AttributeError: '_thread._local' object has no attribute 'hub'

(...)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<frozen importlib._bootstrap>", line 221, in acquire
  File "/home/haypo/prog/eventlet/eventlet/semaphore.py", line 96, in acquire
    hubs.get_hub().switch()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 115, in get_hub
    use_hub()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 77, in use_hub
    mod = get_default_hub()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 45, in get_default_hub
    import eventlet.hubs.epolls
  File "<frozen importlib._bootstrap>", line 295, in _lock_unlock_module
  File "<frozen importlib._bootstrap>", line 211, in acquire
  File "/home/haypo/prog/eventlet/eventlet/semaphore.py", line 103, in __enter__
    self.acquire()
RuntimeError: maximum recursion depth exceeded

(...)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<frozen importlib._bootstrap>", line 221, in acquire
  File "/home/haypo/prog/eventlet/eventlet/semaphore.py", line 96, in acquire
    hubs.get_hub().switch()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 115, in get_hub
    use_hub()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 77, in use_hub
    mod = get_default_hub()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 45, in get_default_hub
    import eventlet.hubs.epolls
  File "<frozen importlib._bootstrap>", line 295, in _lock_unlock_module
  File "<frozen importlib._bootstrap>", line 224, in acquire
KeyError: 140058384674896

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "x.py", line 43, in <module>
    import encodings.idna
  File "<frozen importlib._bootstrap>", line 1569, in _find_and_load
  File "<frozen importlib._bootstrap>", line 237, in release
  File "/home/haypo/prog/eventlet/eventlet/semaphore.py", line 115, in release
    hubs.get_hub().schedule_call_global(0, self._do_acquire)
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 115, in get_hub
    use_hub()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 77, in use_hub
    mod = get_default_hub()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 45, in get_default_hub
    import eventlet.hubs.epolls
  File "<frozen importlib._bootstrap>", line 295, in _lock_unlock_module
  File "<frozen importlib._bootstrap>", line 224, in acquire
KeyError: 140058384674896
Contributor

vstinner commented Nov 27, 2014

Why?

importlib cannot use eventlet locks because event locks need to import modules.

It looks like the first call the import in the thread calls eventlet.event.Event().acquire() which calls get_hub() which executes (indirectly) "import eventlet.hubs.epolls" which retries to acquire the same event and the loop restarts.

The eventlet.hubs.epolls module is required to setup the hub. In my test, the hub is first setup in a thread, not in the main thread. The problem is that the hub is setup to acquire a lock to import a module, but setup the hub has to import a lock, whereas the hub requires to import a second module.

I guess that calling "get_hub()" in a thread is enough to reproduce the issue.

The importlib modules maintains a dictionary of locks using the thread identifier for the key.

Traceback raises by patcher_test without the fix:

AttributeError: '_thread._local' object has no attribute 'hub'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 112, in get_hub
    _threadlocal.Hub
AttributeError: '_thread._local' object has no attribute 'Hub'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 109, in get_hub
    hub = _threadlocal.hub
AttributeError: '_thread._local' object has no attribute 'hub'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 112, in get_hub
    _threadlocal.Hub
AttributeError: '_thread._local' object has no attribute 'Hub'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 109, in get_hub
    hub = _threadlocal.hub
AttributeError: '_thread._local' object has no attribute 'hub'

(...)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<frozen importlib._bootstrap>", line 221, in acquire
  File "/home/haypo/prog/eventlet/eventlet/semaphore.py", line 96, in acquire
    hubs.get_hub().switch()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 115, in get_hub
    use_hub()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 77, in use_hub
    mod = get_default_hub()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 45, in get_default_hub
    import eventlet.hubs.epolls
  File "<frozen importlib._bootstrap>", line 295, in _lock_unlock_module
  File "<frozen importlib._bootstrap>", line 211, in acquire
  File "/home/haypo/prog/eventlet/eventlet/semaphore.py", line 103, in __enter__
    self.acquire()
RuntimeError: maximum recursion depth exceeded

(...)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<frozen importlib._bootstrap>", line 221, in acquire
  File "/home/haypo/prog/eventlet/eventlet/semaphore.py", line 96, in acquire
    hubs.get_hub().switch()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 115, in get_hub
    use_hub()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 77, in use_hub
    mod = get_default_hub()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 45, in get_default_hub
    import eventlet.hubs.epolls
  File "<frozen importlib._bootstrap>", line 295, in _lock_unlock_module
  File "<frozen importlib._bootstrap>", line 224, in acquire
KeyError: 140058384674896

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "x.py", line 43, in <module>
    import encodings.idna
  File "<frozen importlib._bootstrap>", line 1569, in _find_and_load
  File "<frozen importlib._bootstrap>", line 237, in release
  File "/home/haypo/prog/eventlet/eventlet/semaphore.py", line 115, in release
    hubs.get_hub().schedule_call_global(0, self._do_acquire)
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 115, in get_hub
    use_hub()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 77, in use_hub
    mod = get_default_hub()
  File "/home/haypo/prog/eventlet/eventlet/hubs/__init__.py", line 45, in get_default_hub
    import eventlet.hubs.epolls
  File "<frozen importlib._bootstrap>", line 295, in _lock_unlock_module
  File "<frozen importlib._bootstrap>", line 224, in acquire
KeyError: 140058384674896
@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Dec 12, 2014

Contributor

Does it matter that Travis tests failed? I don't think that Travis failed because of my changes.

Contributor

vstinner commented Dec 12, 2014

Does it matter that Travis tests failed? I don't think that Travis failed because of my changes.

@temoto

This comment has been minimized.

Show comment
Hide comment
@temoto

temoto Dec 12, 2014

Member

No worries, CPU limit failure is not representative on Travis, it's because test machine was generally busy.

Member

temoto commented Dec 12, 2014

No worries, CPU limit failure is not representative on Travis, it's because test machine was generally busy.

@temoto

This comment has been minimized.

Show comment
Hide comment
@temoto

temoto Dec 21, 2014

Member

Please, see this modified commit: 539d3b9
Test module is in separate file. I know there's a lot of code that uses inline-to-temporary-file approach. This new style is preferred since it allows test module checks/highlight.

If you are happy with new commit, I'll merge it.

Member

temoto commented Dec 21, 2014

Please, see this modified commit: 539d3b9
Test module is in separate file. I know there's a lot of code that uses inline-to-temporary-file approach. This new style is preferred since it allows test module checks/highlight.

If you are happy with new commit, I'll merge it.

@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Dec 21, 2014

Contributor

Test module is in separate file.

The code looks better like, I prefer your version.

This new style is preferred since it allows test module checks/highlight.

Exactly.

If you are happy with new commit, I'll merge it.

I don't remember why I configured ImportInThread as a daemon thread. You may try to drop "daemon = True" and call "server.join()" just before print("ok").

By the way, you may rename the variable "server" to "thread". I probably used the name "server" because initially the code triggering the bug was much more complex. It was a TCP network server if I remember correctly (the idna encoding is used to encode a socket name, or something like that.)

Contributor

vstinner commented Dec 21, 2014

Test module is in separate file.

The code looks better like, I prefer your version.

This new style is preferred since it allows test module checks/highlight.

Exactly.

If you are happy with new commit, I'll merge it.

I don't remember why I configured ImportInThread as a daemon thread. You may try to drop "daemon = True" and call "server.join()" just before print("ok").

By the way, you may rename the variable "server" to "thread". I probably used the name "server" because initially the code triggering the bug was much more complex. It was a TCP network server if I remember correctly (the idna encoding is used to encode a socket name, or something like that.)

@temoto

This comment has been minimized.

Show comment
Hide comment
@temoto

temoto Dec 22, 2014

Member

Thanks, I've applied your fixes and merged into master.

Member

temoto commented Dec 22, 2014

Thanks, I've applied your fixes and merged into master.

@temoto temoto closed this Dec 22, 2014

donaghmccabe pushed a commit to donaghmccabe/eventlet that referenced this pull request Feb 12, 2015

Donagh McCabe
Fix exception raising in create_connection()
The create_connection() function could raise a
socket.error instance, where the errno was the actual
socket.error instance.

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