Are there any claims as to whether Dependencies is thread safe? I was
working on an odd startup problem one of our scripts has and it turns
out that there seems to be a race condition:
2 threads spawn
Thread 1 tries to reference Foo, hits const_missing
Thread 2 tries to reference Foo, hits const_missing
Thread 1 enters require_or_load (in dependencies.rb) and adds foo.rb
to the loaded set
Thread 2 performs the test on line 247 (if file_path && !
loaded.include?(File.expand_path(file_path))) which fails because
loaded does now contain foo.rb
Thread 1 completes loading of foo.rb, Object.const_defined?(:Foo) now
returns true
Thread 2 hits line 253:
elsif (parent = from_mod.parent) && parent != from_mod &&
! from_mod.parents.any? { |p| p.const_defined?(const_name) }
which is also false since either from_mod is Object (in which case
parent == from_mod) or from_mod.parents contains Object in which case
the second part of the test
Thread 2 then raises name_error.
I''ve added some appropriately placed sleep statements into
dependencies.rb to make this happen systematically rather than it just
being blind luck. In my particular case we can probably work around
this by explicitly requiring stuff, but this may be of interest to
people thinking about thread safety in rails.
Fred
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---
There was a Trac ticket about this: http://dev.rubyonrails.org/ticket/9155 The problem could probably be fixed with one or more carefully placed Thread#exclusive blocks. I don''t think there is a good reason why several threads have to be able to load new constants in parallel. On K, 2008-04-23 at 10:04 +0100, Frederick Cheung wrote:> Are there any claims as to whether Dependencies is thread safe? I was > working on an odd startup problem one of our scripts has and it turns > out that there seems to be a race condition: > > 2 threads spawn > > Thread 1 tries to reference Foo, hits const_missing > Thread 2 tries to reference Foo, hits const_missing > Thread 1 enters require_or_load (in dependencies.rb) and adds foo.rb > to the loaded set > Thread 2 performs the test on line 247 (if file_path && ! > loaded.include?(File.expand_path(file_path))) which fails because > loaded does now contain foo.rb > Thread 1 completes loading of foo.rb, Object.const_defined?(:Foo) now > returns true > Thread 2 hits line 253: > elsif (parent = from_mod.parent) && parent != from_mod && > ! from_mod.parents.any? { |p| p.const_defined?(const_name) } > > which is also false since either from_mod is Object (in which case > parent == from_mod) or from_mod.parents contains Object in which case > the second part of the test > Thread 2 then raises name_error. > > I''ve added some appropriately placed sleep statements into > dependencies.rb to make this happen systematically rather than it just > being blind luck. In my particular case we can probably work around > this by explicitly requiring stuff, but this may be of interest to > people thinking about thread safety in rails. > > > > Fred > > > >--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> Are there any claims as to whether Dependencies is thread safe? I was > working on an odd startup problem one of our scripts has and it turns > out that there seems to be a race condition:We actually discussed this in #rails-contrib earlier today. As it stands there are absolutely no locks or synchronisation in dependencies.rb, so it''s completely thread-dangerous. There are a couple of things we could do: 1) Have a giant ''doing dependencies lock By wrapping a big lock around load_missing_constant and friends so only one thread goes about creating classes at any given time. All the other threads would have to wait for that lock to be released. This would probably be a little difficult to test, but not too difficult. 2) Have an alternative Dependencies mode. Just load everything up front before we start dispatching requests. Both these options were discussed in #9155 like tarmo said. Rather than Thread#exclusive we could use some specific mutexes so other threads can go about their other business. Any takers? -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On 23 Apr 2008, at 10:20, Tarmo Tänav wrote:> > There was a Trac ticket about this: > http://dev.rubyonrails.org/ticket/9155 > > The problem could probably be fixed with one or more carefully placed > Thread#exclusive blocks. I don''t think there is a good reason why > several threads have to be able to load new constants in parallel. >D''oh, so hell bent in working out exactly why my particular problem was happening that I didn''t think to check trac. Fred> > On K, 2008-04-23 at 10:04 +0100, Frederick Cheung wrote: >> Are there any claims as to whether Dependencies is thread safe? I was >> working on an odd startup problem one of our scripts has and it turns >> out that there seems to be a race condition: >> >> 2 threads spawn >> >> Thread 1 tries to reference Foo, hits const_missing >> Thread 2 tries to reference Foo, hits const_missing >> Thread 1 enters require_or_load (in dependencies.rb) and adds foo.rb >> to the loaded set >> Thread 2 performs the test on line 247 (if file_path && ! >> loaded.include?(File.expand_path(file_path))) which fails because >> loaded does now contain foo.rb >> Thread 1 completes loading of foo.rb, Object.const_defined?(:Foo) now >> returns true >> Thread 2 hits line 253: >> elsif (parent = from_mod.parent) && parent != from_mod && >> ! from_mod.parents.any? { |p| p.const_defined? >> (const_name) } >> >> which is also false since either from_mod is Object (in which case >> parent == from_mod) or from_mod.parents contains Object in which case >> the second part of the test >> Thread 2 then raises name_error. >> >> I''ve added some appropriately placed sleep statements into >> dependencies.rb to make this happen systematically rather than it >> just >> being blind luck. In my particular case we can probably work around >> this by explicitly requiring stuff, but this may be of interest to >> people thinking about thread safety in rails. >> >> >> >> Fred >> >> >>> > > > >--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On 23 Apr 2008, at 10:25, Michael Koziarski wrote:> >> Are there any claims as to whether Dependencies is thread safe? I was >> working on an odd startup problem one of our scripts has and it turns >> out that there seems to be a race condition: > > We actually discussed this in #rails-contrib earlier today. As it > stands there are absolutely no locks or synchronisation in > dependencies.rb, so it''s completely thread-dangerous. There are a > couple of things we could do: >Thanks for the confirmation.> 1) Have a giant ''doing dependencies lock > > By wrapping a big lock around load_missing_constant and friends so > only one thread goes about creating classes at any given time. All > the other threads would have to wait for that lock to be released. > This would probably be a little difficult to test, but not too > difficult. >And loading constants isn''t something you spent a lot of time doing (except perhaps in dev mode) so it shouldn''t be much of a bottleneck. The experiments I did around forcing a systematic failure could conceivably be used to write a failing test for the current version of rails. I might have a poke, although there are a few other things that I''ve been meaning to tackle that I haven''t got round to yet. Fred> 2) Have an alternative Dependencies mode. > > Just load everything up front before we start dispatching requests. > > Both these options were discussed in #9155 like tarmo said. Rather > than Thread#exclusive we could use some specific mutexes so other > threads can go about their other business. Any takers? > > > > -- > Cheers > > Koz > > >--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> Both these options were discussed in #9155 like tarmo said. Rather > than Thread#exclusive we could use some specific mutexes so other > threads can go about their other business. Any takers?Of course, as commented in trac, defining a new class isn''t atomic, so it''s probably Thread#exclusive or nothing. Thread#exclusive depends on Thread#critical= which isn''t considered kosher, as a result both are gone from 1.9. So, unless I''m missing something we need to have an alternative Dependencies mode if we want really do anything deterministic with dependencies and threads? -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Apr 23, 11:25 am, "Michael Koziarski" <mich...@koziarski.com> wrote:> 1) Have a giant ''doing dependencies lock > > By wrapping a big lock around load_missing_constant and friends so > only one thread goes about creating classes at any given time. All > the other threads would have to wait for that lock to be released. > This would probably be a little difficult to test, but not too > difficult.This could result in a deadlock, if the loaded .rb file hits const_missing too. As far as I know, Mutex is not recursive. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> This could result in a deadlock, if the loaded .rb file hits > const_missing too. As far as I know, Mutex is not recursive.I assume we could work around this by keeping track of the thread which obtained the lock. However the partially-defined-constant thing just shoots it all to hell :). -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On 23 Apr 2008, at 10:42, Michael Koziarski wrote:> >> Both these options were discussed in #9155 like tarmo said. Rather >> than Thread#exclusive we could use some specific mutexes so other >> threads can go about their other business. Any takers? > > Of course, as commented in trac, defining a new class isn''t atomic, so > it''s probably Thread#exclusive or nothing.Are you worried about Thread 1 hits const_missing, starts to load foo.rb Thread 2 references Foo.something, doesn''t hit const_missing because Foo is now defined, but not all of the methods have been added ? Nasty :-). Can anything ugly like forcing everyting to be loaded into a temporary module (so that no one else sees it) and then ''uncloaking'' when we''re finished. Sounds like it would just be over the top compared to just loading everything. Fred> > > Thread#exclusive depends on Thread#critical= which isn''t considered > kosher, as a result both are gone from 1.9. > > So, unless I''m missing something we need to have an alternative > Dependencies mode if we want really do anything deterministic with > dependencies and threads? > > > -- > Cheers > > Koz > > >--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> Can anything ugly like forcing everyting to be loaded into a temporary > module (so that no one else sees it) and then ''uncloaking'' when we''re > finished. Sounds like it would just be over the top compared to just > loading everything.Yeah, and it''s more an issue with ruby rather than something we should try to work around. After all: class Foo def Foo.some_class_method end end So does anyone want to take a stab at a dependencies mechanism which just loads everything and has regular const_missing behaviour if it hits something it doesn''t understand? -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On 23 Apr 2008, at 11:23, Michael Koziarski wrote:> >> Can anything ugly like forcing everyting to be loaded into a >> temporary >> module (so that no one else sees it) and then ''uncloaking'' when we''re >> finished. Sounds like it would just be over the top compared to just >> loading everything. > > Yeah, and it''s more an issue with ruby rather than something we should > try to work around. After all: > > class Foo > def Foo.some_class_method > end > end > > So does anyone want to take a stab at a dependencies mechanism which > just loads everything and has regular const_missing behaviour if it > hits something it doesn''t understand?I''ll take a shot at it, but I won''t have time for a few days Fred --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Wed, Apr 23, 2008 at 4:46 AM, Hongli Lai <honglilai@gmail.com> wrote:> > > This could result in a deadlock, if the loaded .rb file hits > const_missing too. As far as I know, Mutex is not recursive.Yeah, use Monitor instead which is reentrant. /Nick --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Apr 23, 2008, at 11:25 , Michael Koziarski wrote:> 2) Have an alternative Dependencies mode. > > Just load everything up front before we start dispatching requests.Does anybody have analized and alternative approach which would consist on walking load_path before anything to generate Ruby builtin autoload calls? --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Apr 23, 2008, at 2:42 AM, Michael Koziarski wrote:> >> Both these options were discussed in #9155 like tarmo said. Rather >> than Thread#exclusive we could use some specific mutexes so other >> threads can go about their other business. Any takers? > > Of course, as commented in trac, defining a new class isn''t atomic, so > it''s probably Thread#exclusive or nothing. > > Thread#exclusive depends on Thread#critical= which isn''t considered > kosher, as a result both are gone from 1.9. > > So, unless I''m missing something we need to have an alternative > Dependencies mode if we want really do anything deterministic with > dependencies and threads? > > > -- > Cheers > > KozI''d recommend not using Thread.exclusive at all it will end up limiting rails options on alternate ruby impls. We should either wrap a mutex around any calls to the Dependencies, or even better in production mode we should preload all classes before any hits to the application which would avoid this issue entirely. Cheers- - Ezra Zygmuntowicz -- Founder & Software Architect -- ezra@engineyard.com -- EngineYard.com --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> Does anybody have analized and alternative approach which would > consist on walking load_path before anything to generate Ruby builtin > autoload calls?I believe the mod_rails guys do this for their system, it allows the copy-on-write stuff to only store one copy of all of your apps classes. So there are definitely people trying it out. The difference is that they''re still able to use the const_missing stuff in the child processes. As discussed in ticket #9155 there''s no way this can behave in a deterministic manner with multiple threads requiring classes. So preloading before spawning threads is really the only safe option, taking the stuff from Hongli and co as a starting point -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> I''d recommend not using Thread.exclusive at all it will end up > limiting rails options on alternate ruby impls. We should either wrap > a mutex around any calls to the Dependencies, or even better in > production mode we should preload all classes before any hits to the > application which would avoid this issue entirely.Yeah, we don''t even have Thread.exclusive in 1.9, so that''s out. Unfortunately mutexes can''t save us either, so preloading is the obvious choice. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---