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

connection fails to clean up in-use state after interrupt #542

Closed
tamird opened this issue Sep 9, 2014 · 8 comments · Fixed by #592
Closed

connection fails to clean up in-use state after interrupt #542

tamird opened this issue Sep 9, 2014 · 8 comments · Fixed by #592

Comments

@tamird
Copy link
Contributor

tamird commented Sep 9, 2014

repro case: https://gist.github.com/tamird/a2ead907a70a70d54236
this code reliably produces Mysql2::Error: This connection is still waiting for a result, try again once you have the result on my machine (and we see it in production, too).

I don't know enough about the MRI C API, so I haven't had much luck fixing this issue myself. /cc @eam

@sodabrew
Copy link
Collaborator

sodabrew commented Sep 9, 2014

This is a known issue on Ruby 2.1, they changed the Timeout.timeout exception mechanism to only catch if you provide an explicit class type as the second argument.

Which is to say, in your example, rescue Timeout::Error is not caught. Try putting a puts in there, you'll see that it isn't called.

@tamird
Copy link
Contributor Author

tamird commented Sep 9, 2014

huh? it is caught. also, I've added the reconnect option to the gist and now it is failing reproducibly in Ruby 2.0.0 as well.

@sodabrew
Copy link
Collaborator

sodabrew commented Sep 9, 2014

I have only seen this error on Ruby 2.1, and it is resolved by using the second argument to Timeout.timeout. If you can reproduce it on Ruby 2.0, and/or it is not resolved even with the second argument to Timeout.timeout, then I can imagine that mysql2 will need some fixing. But I'm not sure what fixing is needed.

@tamird
Copy link
Contributor Author

tamird commented Sep 9, 2014

The gist i've provided fails on ruby 2.0.0.

@tamird
Copy link
Contributor Author

tamird commented Sep 9, 2014

I believe the fix needed is to make the cleanup call more resilient -- the connection should either disconnect or be marked inactive

@tamird
Copy link
Contributor Author

tamird commented Mar 10, 2015

@sodabrew ping, this is probably the same issue we "fixed" in mperham/connection_pool#67

Shall I attempt a similar fix?

@sodabrew
Copy link
Collaborator

I read through that issue, and I don't think I get it 🎱 What would you change in mysql2? PRs always welcome!

@tamird
Copy link
Contributor Author

tamird commented Mar 12, 2015

@sodabrew #592

chrisface pushed a commit to notonthehighstreet/mysql2 that referenced this issue Nov 9, 2015
When available, we prevent `Timeout.timeout` from corrupting
connections using `Thread.handle_interrupt`. Fixes brianmario#542.

Revert "Update specs for Ruby 2.1 Timeout behavior by specifying 'Timeout::Error' as the klass parameter."

This reverts commit d0a5199.
sodabrew pushed a commit to sodabrew/mysql2 that referenced this issue May 7, 2016
When available, we prevent `Timeout.timeout` from corrupting
connections using `Thread.handle_interrupt`. Fixes brianmario#542.

Revert "Update specs for Ruby 2.1 Timeout behavior by specifying 'Timeout::Error' as the klass parameter."

This reverts commit d0a5199.
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 a pull request may close this issue.

2 participants