ElMassimo / request_store_rails

📦 Per-request global storage for Rails prepared for multi-threaded apps
https://github.com/ElMassimo/request_store_rails
MIT License
85 stars 6 forks source link

Using hash as class variable in multithread #8

Closed josefsj3m closed 6 years ago

josefsj3m commented 6 years ago

Hi All, my question is not a issue really, but a form of use that can generate an issue if using Puma as application server.

How to handle hash with RequestLocals? See the following example:

class Test
  RequestLocals.store[:some_hash] = {}

  def self.method1
    ...
      RequestLocals.fetch(:some_hash).merge!(1  => '11')
    ...
  end

def self.method2
    ...
      RequestLocals.fetch(:some_hash).delete(1)  <= Imagine two threads running this code at the same time
    ...
  end

end

Does RequestLocals synchronize the access ? I couldn't see a delete method just for one item, RequestLocals.delete(:some_hash) deletes the entire hash.

Thanks,

Fernando.

ElMassimo commented 6 years ago

Hi Fernando, if you need thread-safe access you should use a data structure that is designed to support that, such as Concurrent::Map instead of Hash.

In fact, this library uses a modified Concurrent::Map to manage the stored keys, you will notice that it replaces the write_lock with a Monitor to support re-entrant access (for example, when using fetch with a block that internally also checks or writes to the store).

In most cases, Concurrent::Map will get the job done without any modifications 😃

josefsj3m commented 6 years ago

Hi Fernando, if you need thread-safe access you should use a data structure that is designed to support that, such as Concurrent::Map instead of Hash.

In fact, this library uses a modified Concurrent::Map to manage the stored keys, you will notice that it replaces the write_lock with a Monitor to support re-entrant access (for example, when using fetch with a block that internally also checks or writes to the store).

In most cases, Concurrent::Map will get the job done without any modifications

Thanks @ElMassimo for the answer. So, do you think that is better to use Concurrency::Map in the code? for instance: Change

class Test
  RequestLocals.store[:some_hash] = {}

To

class Test
  RequestLocals.store[:some_hash] = Concurrency::Map.new