steveklabnik / request_store

Per-request global storage for Rack.
https://github.com/steveklabnik/request_store
MIT License
1.47k stars 87 forks source link

Release 1.7.0 -- no longer use Fiber - reverts Fiber changes in release 1.6.0 #99

Closed professor closed 4 months ago

professor commented 4 months ago

Fixes https://github.com/steveklabnik/request_store/issues/96 and Fixes #98

Unless a Fiber is initialized with Fiber.new(storage: nil) then there is cross Fiber pollution of the RequestStore if the RequestStore is initialized or used in any parent Fiber.

Notes

1.6.0 added a method RequestStore.scope -- I was tempted to keep the method in 1.7.0 but decided to remove it for two reasons even though it is a breaking change.

smudge commented 4 months ago

This revert would work for me & my team, thank you. 👍

My only thought would be to add a regression test, something like the one in #97. I can confirm that (on Ruby 3.2+) this fails for me on 1.6 and passes with the changes in this PR:

  def test_concurrent_stores
    Thread.new { RequestStore.store[:foo] = 'bar' }.join
    value = Thread.new { RequestStore.store[:foo] }.value

    assert value != 'bar', 'Thread 2 should not see the value set by Thread 1'
  end
professor commented 4 months ago

Thanks. I'm hoping to add a regression test as a separate PR.

steveklabnik commented 4 months ago

Thank you so much, both of you. <3

steveklabnik commented 4 months ago

https://rubygems.org/gems/request_store/versions/1.7.0