Permalink
Browse files

Correctly unlock the reloader lock if the underlying app raises an ex…

…ception.

[#2873 state:incomplete]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
  • Loading branch information...
1 parent 9284bcc commit a91969803e96b4f66cebd65ac67c8cdc4ca4fcef @FooBarWidget FooBarWidget committed with jeremy Aug 10, 2009
Showing with 32 additions and 15 deletions.
  1. +19 −14 actionpack/lib/action_controller/reloader.rb
  2. +13 −1 actionpack/test/controller/reloader_test.rb
@@ -34,20 +34,25 @@ def initialize(app, lock = @@lock)
def call(env)
@lock.lock
Dispatcher.reload_application
- status, headers, body = @app.call(env)
- # We do not want to call 'cleanup_application' in an ensure block
- # because the returned Rack response body may lazily generate its data. This
- # is for example the case if one calls
- #
- # render :text => lambda { ... code here which refers to application models ... }
- #
- # in an ActionController.
- #
- # Instead, we will want to cleanup the application code after the request is
- # completely finished. So we wrap the body in a BodyWrapper class so that
- # when the Rack handler calls #close during the end of the request, we get to
- # run our cleanup code.
- [status, headers, BodyWrapper.new(body, @lock)]
+ begin
+ status, headers, body = @app.call(env)
+ # We do not want to call 'cleanup_application' in an ensure block
+ # because the returned Rack response body may lazily generate its data. This
+ # is for example the case if one calls
+ #
+ # render :text => lambda { ... code here which refers to application models ... }
+ #
+ # in an ActionController.
+ #
+ # Instead, we will want to cleanup the application code after the request is
+ # completely finished. So we wrap the body in a BodyWrapper class so that
+ # when the Rack handler calls #close during the end of the request, we get to
+ # run our cleanup code.
+ [status, headers, BodyWrapper.new(body, @lock)]
+ rescue Exception
+ @lock.unlock
+ raise
+ end
end
end
end
@@ -66,7 +66,7 @@ def test_it_locks_before_calling_app
assert lock.locked?
end
- def it_unlocks_upon_calling_close_on_body
+ def test_it_unlocks_upon_calling_close_on_body
lock = MyLock.new
Dispatcher.expects(:reload_application)
reloader = Reloader.new(lambda { |env|
@@ -77,6 +77,18 @@ def it_unlocks_upon_calling_close_on_body
assert !lock.locked?
end
+ def test_it_unlocks_if_app_object_raises_exception
+ lock = MyLock.new
+ Dispatcher.expects(:reload_application)
+ reloader = Reloader.new(lambda { |env|
+ raise "oh no!"
+ }, lock)
+ assert_raise(RuntimeError) do
+ reloader.call({ })
+ end
+ assert !lock.locked?
+ end
+
def test_returned_body_object_always_responds_to_close
body = setup_and_return_body(lambda { |env|
[200, { "Content-Type" => "text/html" }, [""]]

1 comment on commit a919698

Can't we use 'ensure' in this situation instead of re-raising an Exception? What's the difference?

Please sign in to comment.