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

Connections can end up in inconsistent state due to except Exception: instead of bare except: #360

Closed
leifkb opened this issue Jun 26, 2013 · 3 comments

Comments

@leifkb
Copy link

leifkb commented Jun 26, 2013

The changelog for 2.7.3 mentions all bare except:s have been changed to except Exception:. Unfortunately, this can sometimes cause connections to be released to the pool in an inconsistent state.

Here's a script that demonstrates the problem using gevent timeouts: https://gist.github.com/leifkb/5871666 And a patch: https://gist.github.com/leifkb/5871691

@andymccurdy
Copy link
Contributor

The problem with the empty except causes is that they swallow KeyboardInterrupt exceptions. This means you can't kill a script that's blocked (likely in a socket call) via ctrl-c / ctrl-break. See #328 for where this change was made.

Do you know why the gevent example breaks? Is there a gevent exception that's being raised that's not inherited from the base Exception class?

@leifkb
Copy link
Author

leifkb commented Jun 26, 2013

Yes, gevent raises Timeout, which is not derived from Exception. The code I'm suggesting changing back to bare except: re-raises the exception, so it shouldn't swallow KeyboardInterrupts. And I think having it not catch KeyboardInterrupt is buggy too -- if a KeyboardInterrupt were raised at the wrong moment, it could cause a connection to get returned to the pool in an inconsistent state too.

@andymccurdy
Copy link
Contributor

Alright, I think this is correct since the exception gets re-raised anyway. Fixed in 80baa99

ikonst added a commit to ikonst/redis-py that referenced this issue Dec 12, 2022
ikonst added a commit to ikonst/redis-py that referenced this issue Dec 12, 2022
ikonst added a commit to ikonst/redis-py that referenced this issue Dec 12, 2022
ikonst added a commit to ikonst/redis-py that referenced this issue Dec 12, 2022
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

No branches or pull requests

2 participants