Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

RequestStore's use of Fiber and whether storage is duplicated across Fibers. #98

Closed
professor opened this issue Apr 30, 2024 · 2 comments · Fixed by #99
Closed

RequestStore's use of Fiber and whether storage is duplicated across Fibers. #98

professor opened this issue Apr 30, 2024 · 2 comments · Fixed by #99

Comments

@professor
Copy link
Contributor

professor commented Apr 30, 2024

I'm debugging an issue in PaperTrail and how it uses RequestStore.

I noticed that one of their tests failed with the switch from Ruby 3.1.5 to 3.2.0:

ruby version rails version status RequestStore.scope
3.0.1 7.1 green
3.1.3 7.1 green
3.1.5 7.1 green Thread
3.2.0 7.1 red Fiber
3.2.4 7.1 red
3.3.1 7.1 red

In the test they assert that a new thread would get a new RequestStore:

Thread.new { expect(described_class.whodunnit).to be_nil }.join
Thread.new { expect(described_class.enabled_for_model?(Widget)).to eq true }.join

I'm able to update the test for RequestStore and Fiber like so:

Fiber.new(storage: nil) { expect(described_class.whodunnit).to be_nil }.resume
Fiber.new(storage: nil) { expect(described_class.enabled_for_model?(Widget)).to eq true }.resume

Notice that when a new Fiber is created, by default it "inherits a copy of the storage from the current fiber."

I realize that RequestStore does not specify how Fibers are created, and this made me wonder if there's a potential bug in using RequestStore since it may be hard to see how new Fibers are created in puma or thin. (Currently puma and thin use Thread.new)

@mattbrictson
Copy link

There is also a discussion in #96 (comment) that I think concerns the same underlying issue. Note that request_store 1.6.0 switched from thread local storage to fiber storage, which is what triggered the problem in my case.

@professor
Copy link
Contributor Author

Thank you @mattbrictson -- let's ammend the issue title in #96 -- I had assumed it wasn't related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants