Sadly, this segv's in 1.8 :(

2 files changed, 61 insertions, 43 deletions

diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb
index 2b80bd214f..38727f77ee 100644
--- a/activesupport/lib/active_support/dependencies.rb
+++ b/activesupport/lib/active_support/dependencies.rb
@@ -63,58 +63,78 @@ module ActiveSupport #:nodoc:
     mattr_accessor :log_activity
     self.log_activity = false
 
-    class WatchStack < Array
+    # The WatchStack keeps a stack of the modules being watched as files are loaded.
+    # If a file in the process of being loaded (parent.rb) triggers the load of 
+    # another file (child.rb) the stack will ensure that child.rb handles the new 
+    # constants.
+    #
+    # If child.rb is being autoloaded, its constants will be added to
+    # autoloaded_constants. If it was being `require`d, they will be discarded.
+    #
+    # This is handled by walking back up the watch stack and adding the constants
+    # found by child.rb to the list of original constants in parent.rb
+    class WatchStack < Hash
+      # @watching is a stack of lists of constants being watched. For instance,
+      # if parent.rb is autoloaded, the stack will look like [[Object]]. If parent.rb
+      # then requires namespace/child.rb, the stack will look like [[Object], [Namespace]].
+
       def initialize
-        @mutex = Mutex.new
+        @watching = []
+        super { |h,k| h[k] = [] }
       end
 
-      def self.locked(*methods)
-        methods.each { |m| class_eval "def #{m}(*) lock { super } end", __FILE__, __LINE__ }
-      end
+      # return a list of new constants found since the last call to watch_modules
+      def new_constants
+        constants = []
 
-      locked :concat, :each, :delete_if, :<<
+        # Grab the list of namespaces that we're looking for new constants under
+        @watching.last.each do |namespace|
+          # Retrieve the constants that were present under the namespace when watch_modules
+          # was originally called
+          original_constants = self[namespace].last
 
-      def new_constants_for(frames)
-        constants = []
-        frames.each do |mod_name, prior_constants|
-          mod = Inflector.constantize(mod_name) if Dependencies.qualified_const_defined?(mod_name)
+          mod = Inflector.constantize(namespace) if Dependencies.qualified_const_defined?(namespace)
           next unless mod.is_a?(Module)
 
-          new_constants = mod.local_constant_names - prior_constants
-
-          # If we are checking for constants under, say, :Object, nested under something
-          # else that is checking for constants also under :Object, make sure the
-          # parent knows that we have found, and taken care of, the constant.
-          #
-          # In particular, this means that since Kernel.require discards the constants
-          # it finds, parents will be notified that about those constants, and not
-          # consider them "new". As a result, they will not be added to the
-          # autoloaded_constants list.
-          each do |key, value|
-            value.concat(new_constants) if key == mod_name
+          # Get a list of the constants that were added
+          new_constants = mod.local_constant_names - original_constants
+
+          # self[namespace] returns an Array of the constants that are being evaluated
+          # for that namespace. For instance, if parent.rb requires child.rb, the first
+          # element of self[Object] will be an Array of the constants that were present
+          # before parent.rb was required. The second element will be an Array of the
+          # constants that were present before child.rb was required.
+          self[namespace].each do |constants|
+            constants.concat(new_constants)
           end
 
+          # Normalize the list of new constants, and add them to the list we will return
           new_constants.each do |suffix|
-            constants << ([mod_name, suffix] - ["Object"]).join("::")
+            constants << ([namespace, suffix] - ["Object"]).join("::")
           end
         end
         constants
+      ensure
+        # A call to new_constants is always called after a call to watch_modules
+        pop_modules(@watching.pop)
       end
 
       # Add a set of modules to the watch stack, remembering the initial constants
-      def add_modules(modules)
-        list = modules.map do |desc|
-          name = Dependencies.to_constant_name(desc)
-          consts = Dependencies.qualified_const_defined?(name) ?
-            Inflector.constantize(name).local_constant_names : []
-          [name, consts]
+      def watch_namespaces(namespaces)
+        watching = []
+        namespaces.map do |namespace|
+          module_name = Dependencies.to_constant_name(namespace)
+          original_constants = Dependencies.qualified_const_defined?(module_name) ?
+            Inflector.constantize(module_name).local_constant_names : []
+
+          watching << module_name
+          self[module_name] << original_constants
         end
-        concat(list)
-        list
+        @watching << watching
       end
 
-      def lock
-        @mutex.synchronize { yield self }
+      def pop_modules(modules)
+        modules.each { |mod| self[mod].pop }
       end
     end
 
@@ -563,14 +583,15 @@ module ActiveSupport #:nodoc:
     # and will be removed immediately.
     def new_constants_in(*descs)
       log_call(*descs)
-      watch_frames = constant_watch_stack.add_modules(descs)
 
+      constant_watch_stack.watch_namespaces(descs)
       aborting = true
+
       begin
         yield # Now yield to the code that is to define new constants.
         aborting = false
       ensure
-        new_constants = constant_watch_stack.new_constants_for(watch_frames)
+        new_constants = constant_watch_stack.new_constants
 
         log "New constants: #{new_constants * ', '}"
         return new_constants unless aborting
@@ -580,9 +601,6 @@ module ActiveSupport #:nodoc:
       end
 
       return []
-    ensure
-      # Remove the stack frames that we added.
-      watch_frames.each {|f| constant_watch_stack.delete(f) } if watch_frames.present?
     end
 
     class LoadingModule #:nodoc:
diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb
index f98d823f00..77b885dc3d 100644
--- a/activesupport/test/dependencies_test.rb
+++ b/activesupport/test/dependencies_test.rb
@@ -576,14 +576,14 @@ class DependenciesTest < Test::Unit::TestCase
 
   def test_new_contants_in_without_constants
     assert_equal [], (ActiveSupport::Dependencies.new_constants_in(Object) { })
-    assert ActiveSupport::Dependencies.constant_watch_stack.empty?
+    assert ActiveSupport::Dependencies.constant_watch_stack.all? {|k,v| v.empty? }
   end
 
   def test_new_constants_in_with_a_single_constant
     assert_equal ["Hello"], ActiveSupport::Dependencies.new_constants_in(Object) {
                               Object.const_set :Hello, 10
                             }.map(&:to_s)
-    assert ActiveSupport::Dependencies.constant_watch_stack.empty?
+    assert ActiveSupport::Dependencies.constant_watch_stack.all? {|k,v| v.empty? }
   ensure
     Object.class_eval { remove_const :Hello }
   end
@@ -600,7 +600,7 @@ class DependenciesTest < Test::Unit::TestCase
     end
 
     assert_equal ["OuterAfter", "OuterBefore"], outer.sort.map(&:to_s)
-    assert ActiveSupport::Dependencies.constant_watch_stack.empty?
+    assert ActiveSupport::Dependencies.constant_watch_stack.all? {|k,v| v.empty? }
   ensure
     %w(OuterBefore Inner OuterAfter).each do |name|
       Object.class_eval { remove_const name if const_defined?(name) }
@@ -621,7 +621,7 @@ class DependenciesTest < Test::Unit::TestCase
       M.const_set :OuterAfter, 30
     end
     assert_equal ["M::OuterAfter", "M::OuterBefore"], outer.sort
-    assert ActiveSupport::Dependencies.constant_watch_stack.empty?
+    assert ActiveSupport::Dependencies.constant_watch_stack.all? {|k,v| v.empty? }
   ensure
     Object.class_eval { remove_const :M }
   end
@@ -639,7 +639,7 @@ class DependenciesTest < Test::Unit::TestCase
       M.const_set :OuterAfter, 30
     end
     assert_equal ["M::OuterAfter", "M::OuterBefore"], outer.sort
-    assert ActiveSupport::Dependencies.constant_watch_stack.empty?
+    assert ActiveSupport::Dependencies.constant_watch_stack.all? {|k,v| v.empty? }
   ensure
     Object.class_eval { remove_const :M }
   end