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

ConnectionManagement middleware drains connection pool when Exceptions are raised #11497

Closed
orslumen opened this issue Jul 18, 2013 · 3 comments · Fixed by #11538
Closed

ConnectionManagement middleware drains connection pool when Exceptions are raised #11497

orslumen opened this issue Jul 18, 2013 · 3 comments · Fixed by #11538

Comments

@orslumen
Copy link

As ActiveRecord::ConnectionAdapters::ConnectionManagement middleware does not rescue from Exception (but only from StandardError), the Connection Pool quickly runs out of connections when multiple erroneous Requests come in right after each other.

class TestController < ActionController::Base
  def eat_connection
    # do something with the DB
    raise Exception('lost a connection')
  end

  def release_all_connections
    head :ok
  end
end

Call /test/eat_connection as many times as you have connections in your pool and the application becomes unresponsive, until the connections timeout and are returned to the pool.

Call /test/eat_connection a few times, followed by /test/release_all_connections and the entire pool becomes available again. This is probably what happens with most applications, but it is risky imo that someone who knows how to cause an Exception could potentially bring down the entire application.

Was it a conscious choice to write rescue instead of rescue Exception?

@senny
Copy link
Member

senny commented Jul 19, 2013

The relevant commit is 3b2a032 . It could be an oversight. Let's wait for @tenderlove to elaborate.

@thedarkone
Copy link
Contributor

Yep, plain rescue not catching all exceptions has bitten me multiple times :( (wish Matz wouldn't have made it this way). We should check the rest of the Rails code base for any "naked" rescue clauses.

@tenderlove
Copy link
Member

Yep, this is a bug. We need to rescue from Exception rather than a naked rescue.

vipulnsward added a commit to vipulnsward/rails that referenced this issue Jul 22, 2013
Fixes rails#11497

As `ActiveRecord::ConnectionAdapters::ConnectionManagement` middleware does not rescue from Exception (but only from StandardError),
the Connection Pool quickly runs out of connections when multiple erroneous Requests come in right after each other.

Recueing from all exceptions and not just StandardError, fixes this behaviour.
thedarkone added a commit to thedarkone/rails that referenced this issue Aug 6, 2013
ehtb pushed a commit to ehtb/rails that referenced this issue Aug 7, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants