From: Aaron Patterson Date: 2012-10-27T02:36:04+09:00 Subject: [ruby-core:48312] Re: [ruby-trunk - Bug #7097] Thread locals don't work inside Enumerator On Fri, Oct 26, 2012 at 11:38:19PM +0900, Eregon (Benoit Daloze) wrote: > > Issue #7097 has been updated by Eregon (Benoit Daloze). > > > tenderlovemaking (Aaron Patterson) wrote: > > I spoke with ko1-san and Usa-san last night, and we thought that thread_variable_(get|set) would be good (similar to instance_variable_(get|set)). I've attached an updated patch to make that change. The documentation includes examples of how the thread local storage and fiber local storage are different. > > > > I added two more methods: > > > > * Thread#thread_variables # => returns a list of the defined variable keys > > * Thread#thread_variable? # => returns true if a key is set, otherwise false > > > > Thread#local_variable_(get|set) methods respect the same security and frozen behavior as Thread#[] and Thread#[]=. > > Sounds great, but I feel the "thread_" prefix is redundant given it is called on a Thread object. > I'm thinking to two alternatives: > > * Remove the "thread_" prefix (as usually the thread instance is Thread.current which is very explicit, or the variable name should be clear enough). > > Thread.current.variable_get(:var) > thread.variable_get(:var) > worker.variable_get(:var) > # versus > Thread.current.thread_variable_get(:var) > thread.thread_variable_get(:var) > worker.thread_variable_get(:var) > > Also, I think get/set do not feel very ruby-like, so ... If it becomes cumbersome, we can add aliases later. get/set aren't very ruby-like, but we have other examples (instance_variable_(get|set)). > * Use a API resembling fiber locals: > > Thread#locals # => returns an object responding to #[] and #[]= (and maybe #variables and #include?) > > Thread.current.locals[:var] = some_value > Thread.current.locals[:var] > > I guess exposing the whole Hash is exposing internal structures and so is unreasonable. But doing so would be intuitive and avoid having 4 new methods for 4 well-known ([],[]=,keys,include?). I'd rather not expose another object. We can't just expose the hash object because it would have inconsistent behavior with fiber locals: irb(main):001:0> t = Thread.new { }.join => # irb(main):002:0> t[:foo] = "bar" => "bar" irb(main):003:0> t["foo"] => "bar" irb(main):004:0> Also, we'd have to figure out what calling `freeze` on that hash means. Because of these things, we would have to expose a new object that isn't a Hash. Exposing a new object means propagating things like Thread#freeze down to the new object. If we implement these 4 new method in my patch, our API footprint is only increased by 4 new methods (vs 5 methods + a new object type). If we find later on that exposing an object is a good thing, we can easily implement these 4 methods in terms of the new object and deprecate the 4 methods. Going in the other direction, deprecating an object, is not so easy. I don't particularly care what the method names are (since we can just alias them later), but I'm firmly against exposing a new object. -- Aaron Patterson http://tenderlovemaking.com/