Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #7579 (closed defect: fixed)

Opened 1 year ago

Last modified 8 months ago

[PATCH] Rails calls {}.requires_reloading? when allow_concurrency = true

Reported by: qertoip Assigned to: core
Priority: normal Milestone: 1.2.6
Component: ActiveRecord Version: 1.2.3
Severity: major Keywords: allow_concurrency requires_reloading? active_connections thread_safe_active_connections single_threaded_active_connections connection_specification.rb
Cc: masterkain

Description

Rails calls #requires_reloading? on empty Hash when allow_concurrency is set to true. This results in 'method missing'.

How to reproduce a bug:

1) Create brand new app with Rails 1.2.1 or 1.2.2 rails bug

2) Add to environment.rb initializer block: config.active_record.allow_concurrency = true

3) Start server script/server

4) Go here (just example): http://localhost:3000/rails/info/properties

5) See the following backtrace in logs:

Error calling Dispatcher.dispatch #<NoMethodError: undefined method `requires_reloading?' for {}:Hash> d:/ruby185/lib/ruby/gems/1.8/gems/activerecord-1.15.2/lib/active_record/connecti on_adapters/abstract/connection_specification.rb:93:in `clear_reloadable_connect ions!' d:/ruby185/lib/ruby/gems/1.8/gems/activerecord-1.15.2/lib/active_record/connecti on_adapters/abstract/connection_specification.rb:92:in `each' d:/ruby185/lib/ruby/gems/1.8/gems/activerecord-1.15.2/lib/active_record/connecti on_adapters/abstract/connection_specification.rb:92:in `clear_reloadable_connect ions!' d:/ruby185/lib/ruby/gems/1.8/gems/rails-1.2.2/lib/dispatcher.rb:65:in `reset_app lication!' d:/ruby185/lib/ruby/gems/1.8/gems/rails-1.2.2/lib/dispatcher.rb:116:in `reset_af ter_dispatch' d:/ruby185/lib/ruby/gems/1.8/gems/rails-1.2.2/lib/dispatcher.rb:51:in `dispatch'

...

Attachments

ticket_7579_against_6538.diff (1.3 kB) - added by wilson on 04/19/07 17:19:37.
Patch against rev 6538 of trunk

Change History

04/19/07 17:19:07 changed by wilson

When 'allow concurrency' is true, 'active_connections' has the side effect of populating the connection cache with an empty Hash. Presumably this isn't a problem in production mode, but the 'reload connections' code for dev mode does not have conditional behavior for 'allow_concurrency' like most of the other methods in that class.

This simple patch against trunk fixes it for me. P.S. It's probably not a good idea to set 'allow_concurrency' in development mode, or, uhh.. at all.

04/19/07 17:19:37 changed by wilson

  • attachment ticket_7579_against_6538.diff added.

Patch against rev 6538 of trunk

05/05/07 22:11:26 changed by watson

  • summary changed from Rails calls {}.requires_reloading? when allow_concurrency = true to [PATCH] Rails calls {}.requires_reloading? when allow_concurrency = true.

06/01/07 02:05:15 changed by zoran.rilak

  • keywords changed from allow_concurrency requires_reloading? to allow_concurrency requires_reloading? active_connections thread_safe_active_connections single_threaded_active_connections connection_specification.rb.
  • version changed from 1.2.1 to 1.2.3.

I may be wrong, but looking at connection_adapters/abstract/connection_specification.rb it seems that most of the @@active_connection references scattered throughout the code are a leftover from the time before threading support was introduced. There's a pair of methods, thread_safe_active_connections and single_threaded_active_connections, and one of those gets aliased to active_connections depending on the @@allow_concurrency flag. I've replaced all the occurences of @@active_connections (except those inside accessor methods and the initializer) with active_connections and it works just fine.

Can't be bothered to submit a patch; this is a conceptual glitch anyway.

06/16/07 20:04:44 changed by masterkain

  • cc set to masterkain.

07/09/07 21:19:01 changed by jrochkind

I have run into this too. Patch would be appreciated.

07/10/07 17:22:00 changed by jrochkind

For the record, wilson's diff appears to work. The approach suggested by zoran.rilak does not seem to; it suppressed the exception mentioned here, but caused others to raise. wilson's patch appears to solve the problem with no side effects.

07/10/07 17:49:27 changed by zoran.rilak

Yes, jrochkind put it succintly. My thought that all mentions of @@active_connections were in the context of single-threaded model was false; some methods actually use it in a thread-aware context and can differentiate between the two. I'm using wilson's patch now and it works for me.

10/25/07 08:09:44 changed by nzkoz

  • status changed from new to closed.
  • resolution set to fixed.

(In [8014]) Make clear_reloadable_connections! take account of @@allow_concurrency. Closes #7579 [wilson]