Skip to content
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 context.term() deadlock #23

Merged
merged 3 commits into from Jan 6, 2012
Merged

Fix context.term() deadlock #23

merged 3 commits into from Jan 6, 2012

Conversation

zacharyvoase
Copy link
Contributor

This pull request fixes issue #22. See that issue page for comprehensive details.

This change fixes a deadlock when calling context.term() while socket
operations are still ongoing in other greenlets. It allows errors from
the `getsockopt()` call triggered by state changes to propagate into the
waiting greenlet.
The overridden `Socket.close()` was inspecting `self.closed`, which
resulted in a call to `zmq_getsockopt()`, which was raising the `ETERM`
again. It now uses the `self._closed` attribute instead of the property,
which doesn't seem to have any adverse effects on socket-closing
semantics.
If the context is terminated just at the right time, the
__state_changed() method might be in progress, and when it checks
`self.closed` it will raise an `ETERM` in the event loop (again, rather
than the waiting greenlet). I've fixed it by putting the check in the
same try/except as the `getsockopt()` call.
@tmc
Copy link
Owner

tmc commented Jan 6, 2012

Zachary, thank you for your work here, using AsyncResult is clearly more reasonable. Merging!

tmc added a commit that referenced this pull request Jan 6, 2012
Fix context.term() deadlock, switch to AsyncResult for improved exception handling.
@tmc tmc merged commit c599ddf into tmc:master Jan 6, 2012
@tmc tmc mentioned this pull request Jan 6, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants