From: "Eregon (Benoit Daloze)" Date: 2022-10-17T11:49:36+00:00 Subject: [ruby-core:110363] [Ruby master Bug#19062] Introduce `Fiber#locals` for shared inheritable state. Issue #19062 has been updated by Eregon (Benoit Daloze). ioquatix (Samuel Williams) wrote in #note-17: > > It would be a semantic bug if adding a new local in a child fiber affects a parent fiber as @byroot (Jean Boussier) said. > > I don't agree this is a semantic bug. Ruby doesn't have any existing data structures which behave like this, and Ruby is fundamentally a mutable language. I would think all other languages with similar functionality don't have new locals in the parent/child leaking to the other one. Am I wrong? Also that updating a local affects both and not only one of them. > No, I think this is a bad idea, not only is it expensive, it's not a well defined operation in Ruby, and prevents lots of useful sharing, like connection pools. Right, probably not necessary for these locals to copy the values. > > I'm not sure what's the advantage of this proposal vs #19058. In both cases it's O(locals) per new fiber if using a hash. > > No, it's not. This is O(1) per fiber creation, and O(hash dup) per thread creation. So it's more efficient. Only if sharing the Hash semantics which seems very counter-intuitive. > There are no thread safety concerns when inheriting the locals by a fiber Only if it's a Fiber of the same Thread. And also it's invalid e.g. if sharing a DB connection (using some fd) and the Fiber scheduler, e.g. it can mismatch requests/responses. Making the DB connections thread/concurrency-safe seems difficult (most don't), and would lose the point of it being fiber/thread-local and not needing such synchronization. > > Isn't that always a bad idea? Sharing e.g. a DB connection between threads would be unsafe. > > No, it's not bad to share a DB connection between threads, if the DB connection itself is thread safe, it's totally fine. It's also more likely you'd be sharing a connection pool (which in most Ruby projects are unfortunately just globals). What's a concrete scenario where the connection pool should be inherited like that vs global and it would be necessary? > > If we allow inheriting across threads then at least it should be very explicit. > > I don't agree with this, the entire point of this model is to have implicit sharing of important things, e.g. `request_id`, `connection_pool`, `trace_id`, etc. Being explicit is more likely to ensure that these fields are not propagated correctly. It seems extremely unsafe to inherit by default across threads to me. I'd think 99% of what you want to solve here is inheriting between fibers and notably the Enumerator fibers, right? ---------------------------------------- Bug #19062: Introduce `Fiber#locals` for shared inheritable state. https://bugs.ruby-lang.org/issues/19062#change-99656 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN ---------------------------------------- After exploring , I felt uncomfortable about the performance of copying lots of inheritable attributes. Please review that issue for the background and summary of the problem. ## Proposal Introduce `Fiber#locals` which is a hash table of local attributes which are inherited by child fibers. ```ruby Fiber.current.locals[:x] = 10 Fiber.new do pp Fiber.current.locals[:x] # => 10 end ``` It's possible to reset `Fiber.current.locals`, e.g. ```ruby def accept_connection(peer) Fiber.new(locals: nil) do # This causes a new hash table to be allocated. # Generate a new request id for all fibers nested in this one: Fiber[:request_id] = SecureRandom.hex(32) @app.call(env) end.resume end ``` A high level overview of the proposed changes: ```ruby class Fiber def initialize(..., locals: Fiber.current.locals) @locals = locals || Hash.new end attr_accessor :locals def self.[] key self.current.locals[key] end def self.[]= key, value self.current.locals[key] = value end end ``` See the pull request for the full proposed implementation. ## Expected Usage Currently, a lot of libraries use `Thread.current[:x]` which is unexpectedly "fiber local". A common bug shows up when lazy enumerators are used, because it may create an internal fiber. Because `locals` are inherited, code which uses `Fiber[:x]` will not suffer from this problem. Any program that uses true thread locals for per-request state, can adopt the proposed `Fiber#locals` and get similar behaviour, without breaking on per-fiber servers like Falcon, because Falcon can "reset" `Fiber.current.locals` for each request fiber, while servers like Puma won't have to do that and will retain thread-local behaviour. Libraries like ActiveRecord can adopt `Fiber#locals` to avoid the need for users to opt into different "IsolatedExecutionState" models, since it can be transparently handled by the web server (see for more details). We hope by introducing `Fiber#locals`, we can avoid all the confusion and bugs of the past designs. -- https://bugs.ruby-lang.org/ Unsubscribe: