Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Configurable recursion tracking causes errors in multi-threaded environment #2897

Closed
andyentity opened this issue Jun 26, 2017 · 0 comments
Closed
Milestone

Comments

@andyentity
Copy link

andyentity commented Jun 26, 2017

How to reproduce:

Configure a column in the list view with a formatted_value definition that takes a little longer to process.

config.model Rollout do
  list do
...
    field :leads do
      formatted_value do
        sleep 0.5
        bindings[:view].render partial: 'rails_admin/main/leads', locals: { object: bindings[:object] }
      end
    end
...
  end
end

The longer it takes the more likely for the race-condition to show up. Run the admin panel in a multi-threaded environment, e.g. using puma in production. Go to the respective list in the admin panel and stress it by reloading the page many times. The best thing is if you create two scopes and always click between the two.

At some point you will get:

NoMethodError at /rollout
undefined method `leads' for #<Rollout:0x007f4a878dfb20>

If you want to use a RailsAdmin virtual field(= a field without corresponding instance method),
you should declare 'formatted_value' in the field definition.
  field :leads do
    formatted_value{ bindings[:object].call_some_method }
  end

Cause:

The recursion tracking in the Configurable module, here.
Doing instance_variable_set("@#{option_name}_recurring", true) is not thread-safe.

  • In this case thread A sets @formatted_value_recurring to true and then evaluates the proc.
  • Meanwhile thread B enters the stage, sees that @formatted_value_recurring is true and thus attempts to call the instance method of the model, which is not there.

Solution:

Scope the recursion tracking variable to the request. This proof-of-concept uses RequestStore.

  if RequestStore.store["rails_admin/#{option_name}_recurring"]
    value = instance_eval(&default)
  else
    RequestStore.store["rails_admin/#{option_name}_recurring"] = true
    value = instance_eval(&value)
    RequestStore.store["rails_admin/#{option_name}_recurring"] = false
  end

The key is not optimal but this proof-of-concept solves the bug on this page.

@andyentity andyentity changed the title Configurable recursion tracking leads to errors in multi-threaded environment Configurable recursion tracking causes errors in multi-threaded environment Jun 26, 2017
@mshibuya mshibuya added this to the 2.0.0 milestone May 21, 2019
adhimos pushed a commit to adhimos/rails_admin that referenced this issue Feb 18, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants