body.close never called on 404 in rack cascade

[cwjenkins]

Intro:
I'm brand new to RoR (come from c/c++/java) and just picked up someone's project that uses nginx/passenger+rack and a gem called client_side_validations (3.1.4). A quick overview of how client side validations works is when you apply uniqueness to a model attribute (example: username) and then state in the view :validate => true it will apply an onchange function to do an ajax call on the form.

The problem:
This ajax call then gets passed to passenger -> rack -> client side validation and returns '200' if the username has already been taken (FOUND) or '404' if the username has not been taken (NOT FOUND). At this point (in client_side_validation) body does not respond to close, it is when it gets to query_cache its wrapped in BodyProxy (it does respond to close) and then in rack cascade it catches '404' which doesn't pass it back to passenger's ensure clause to close the body. Since there is another app in the cascade it gets returned and the body is never closed from the client_side_validation 404 causing the thread deadlock error that I've seen in other middleware that does not close the body (unlock the mutex) "Exception ThreadError in application (deadlock; recursive locking)".

Workaround/fix?:
What I've done to make it work is add the following in cascade.rb

  def call(env)
       result = NotFound

  @apps.each do |app|
    result = app.call(env)
    break unless @catch.include?(result[0].to_i)
    result[2].close if result[2].respond_to? :close  # Should occur after the break. Please check issue #468 by brucek for better solution.
  end

  result
end

Which attempts to close the body regardless. I'm not sure if this is the correct way or maybe this project isn't setup correctly, but this one liner doesn't cause the deadlock issue.

Recap:
If you have middleware that returns a '404' to cascade and leaves the body open with another app left in the app array to be called in cascade then the body associated with the '404' will not be closed causing a deadlock error.

[brucek]

I had this same issue & also spent a while jumping into the Rack deep end to track it down - I just submitted #468 which I think is a cleaner fix since it only closes bodies that are discarded, while not closing bodies that are returned from the call.

[cwjenkins]

Hey Bruce, I appreciate that. I'm still learning ruby/rails so now I see why it should be applied after the break. I hope I helped.
Thanks again,
Colton

[oscardelben]

I'm having trouble reproducing the issue. @brucek, @cwjenkins can you provide a simple rack app that reproduces the problem? Here's what I've tried so far (which works fine): https://gist.github.com/4291781

[cwjenkins]

Hey @oscardelben , unfortunately I'm still learning RoR so I can't provide a 'simple' rack app. What I can do is tell you why yours works and ours does not. In your example you invoke Rack::Builder.new which will invoke '@use << proc { |app| middleware.new(app, *args, &block) }' due to 'use Rack::Lock'. Notice the middleware.new. This in turn invokes Rack::Lock#initialize every time instantiating a new mutex object. Instantiating a new mutex object must produce it on it's own thread because if(mutex->th == GET_THREAD) does not evaulate to true. Now if you'd like I can provide a simple hack that will show you our issue while running your gist. In Rack::Lock

FLAG = 'rack.multithread'.freeze
$locks = []

def initialize(app, mutex = Mutex.new)
  @app, @mutex = app, mutex
end

def call(env)
  old, env[FLAG] = env[FLAG], false
  $locks.push(@mutex)
  @mutex = $locks[0]
  @mutex.lock
  response = @app.call(env)
  body = BodyProxy.new(response[2]) { @mutex.unlock}
  response[2] = body
  response
ensure
  @mutex.unlock unless body
  env[FLAG] = old
end

Then of course if you apply result[2].close if result[2].respond_to?(:close) after the break in Rack::Cascade you will not get the deadlock.
Hope this helps,

Colton

[cwjenkins]

Just learned how. https://gist.github.com/4313869 . Make sure to run it twice! @oscardelben

[raggi]

Fixed in 531384f

[cwjenkins]

@raggi thanks!