Fixed #2294 - Classes sometimes cannot be found

This patch should fix the race condition causing ticket 2294; it extends the loading logic so that:

    * initial load attempts are processed (as before),
    * recursive load attempts return immediately (as before),
    * but subsequent concurrent load attempts from different threads
      wait on a semaphore (condition variable) and then retry (e.g.
      use the now-valid results of the first thread).

This is a slight modification of the solution I'd originally proposed, to prevent a deadlock
that could have arisen if three or more threads simultaneously attempted to load the same item.
Though it solves the bug as reported, it has room for improvement:

    * Failures aren't cached, so repeated attempts will be made to
      import invalid items each time they are encountered
    * It doesn't address any of the underlying referential ambiguity
      (module vs. filename)
    * The threading logic should probably be refactored into a separate
      class (as a start I encapsulated it in an ad hoc singleton class,
      so at least it isn't cluttering up the load method)

Signed-off-by: Markus Roberts <Markus@reality.com>

1 files changed, 71 insertions, 1 deletions

diff --git a/spec/unit/parser/parser.rb b/spec/unit/parser/parser.rb
index bd12f7155..75d0c05a3 100755
--- a/spec/unit/parser/parser.rb
+++ b/spec/unit/parser/parser.rb
@@ -328,4 +328,74 @@ describe Puppet::Parser do
             @parser.version.should == "output"
         end
     end
- end
+
+    describe Puppet::Parser,"when looking up definitions" do
+        it "should check for them by name" do
+            @parser.stubs(:find_or_load).with("namespace","name",:definition).returns(:this_value)
+            @parser.find_definition("namespace","name").should == :this_value
+        end
+    end
+
+    describe Puppet::Parser,"when looking up hostclasses" do
+        it "should check for them by name" do
+            @parser.stubs(:find_or_load).with("namespace","name",:hostclass).returns(:this_value)
+            @parser.find_hostclass("namespace","name").should == :this_value
+        end
+    end
+
+    describe Puppet::Parser,"when looking up names" do
+        before :each do
+            @loaded_code = mock 'loaded code'
+            @loaded_code.stubs(:find_my_type).with('Loaded_namespace',  'Loaded_name').returns(true)
+            @loaded_code.stubs(:find_my_type).with('Bogus_namespace',   'Bogus_name' ).returns(false)
+            @parser = Puppet::Parser::Parser.new :environment => "development",:loaded_code => @loaded_code
+        end
+
+        describe "that are already loaded" do
+            it "should not try to load anything" do
+                @parser.expects(:load).never
+                @parser.find_or_load("Loaded_namespace","Loaded_name",:my_type)
+            end
+            it "should return true" do
+                @parser.find_or_load("Loaded_namespace","Loaded_name",:my_type).should == true
+            end
+        end
+
+        describe "that aren't already loaded" do
+            it "should first attempt to load them with the fully qualified name" do
+                @loaded_code.stubs(:find_my_type).with("Foo_namespace","Foo_name").returns(false,true,true)
+                @parser.expects(:load).with("Foo_namespace::Foo_name").returns(true).then.raises(Exception)
+                @parser.find_or_load("Foo_namespace","Foo_name",:my_type).should == true
+            end
+
+            it "should next attempt to load them with the namespace" do
+                @loaded_code.stubs(:find_my_type).with("Foo_namespace","Foo_name").returns(false,false,true,true)
+                @parser.expects(:load).with("Foo_namespace::Foo_name").returns(false).then.raises(Exception)
+                @parser.expects(:load).with("Foo_namespace").returns(true).then.raises(Exception)
+                @parser.find_or_load("Foo_namespace","Foo_name",:my_type).should == true
+            end
+
+            it "should return false if the name isn't found" do
+                @parser.stubs(:load).returns(false)
+                @parser.find_or_load("Bogus_namespace","Bogus_name",:my_type).should == false
+            end
+        end
+    end
+
+    describe Puppet::Parser,"when loading classnames" do
+        before :each do
+            @loaded_code = mock 'loaded code'
+            @parser = Puppet::Parser::Parser.new :environment => "development",:loaded_code => @loaded_code
+        end
+
+        it "should just return false if the classname is empty" do
+            @parser.expects(:import).never
+            @parser.load("").should == false
+        end
+
+        it "should just return true if the item is loaded" do
+            pending "Need to access internal state (@parser's @loaded) to force this"
+            @parser.load("").should == false
+        end
+    end
+end