The import function was calling type_loader#import directly so that it could pass in the current file name, but by doing so it was thwarting the thread- safety locking level. This patch rearanges things so that all imports go through the same (thread safe) code path while retaining the current_file passing, error handling, etc. from the old structure.
Signed-off-by: Markus Roberts <[email protected]> --- lib/puppet/parser/parser_support.rb | 2 +- lib/puppet/parser/type_loader.rb | 28 +++++++++++++--------------- spec/unit/parser/type_loader_spec.rb | 26 +++++++++++++------------- 3 files changed, 27 insertions(+), 29 deletions(-) diff --git a/lib/puppet/parser/parser_support.rb b/lib/puppet/parser/parser_support.rb index 4f3a4dd..c0fd371 100644 --- a/lib/puppet/parser/parser_support.rb +++ b/lib/puppet/parser/parser_support.rb @@ -111,7 +111,7 @@ class Puppet::Parser::Parser end def import(file) - known_resource_types.loader.import(file, @lexer.file) + known_resource_types.loader.import_if_possible(file, @lexer.file) end def initialize(env) diff --git a/lib/puppet/parser/type_loader.rb b/lib/puppet/parser/type_loader.rb index cb8657f..c33f90d 100644 --- a/lib/puppet/parser/type_loader.rb +++ b/lib/puppet/parser/type_loader.rb @@ -70,7 +70,7 @@ class Puppet::Parser::TypeLoader def initialize(env) self.environment = env - @loaded = [] + @loaded = {} @loading = Helper.new @imported = {} @@ -79,10 +79,13 @@ class Puppet::Parser::TypeLoader def load_until(namespaces, name) return nil if name == "" # special-case main. name2files(namespaces, name).each do |filename| - modname = nil - import_if_possible(filename) do - modname = import(filename) - @loaded << filename + modname = begin + import_if_possible(filename) + rescue Puppet::ImportError => detail + # We couldn't load the item + # I'm not convienced we should just drop these errors, but this + # preserves existing behaviours. + nil end if result = yield(filename) Puppet.info "Automatically imported #{name} from #{filename}" @@ -124,23 +127,18 @@ class Puppet::Parser::TypeLoader parser.parse end - private - # Utility method factored out of load for handling thread-safety. # This isn't tested in the specs, because that's basically impossible. - def import_if_possible(file, &blk) - return if @loaded.include?(file) - begin + def import_if_possible(file, current_file = nil) + @loaded[file] || begin case @loading.owner_of(file) when :this_thread - return + nil when :another_thread - return import_if_possible(file, &blk) + import_if_possible(file,current_file) when :nobody - blk.call + @loaded[file] = import(file,current_file) end - rescue Puppet::ImportError => detail - # We couldn't load the item ensure @loading.done_with(file) end diff --git a/spec/unit/parser/type_loader_spec.rb b/spec/unit/parser/type_loader_spec.rb index db72a23..8f005d5 100644 --- a/spec/unit/parser/type_loader_spec.rb +++ b/spec/unit/parser/type_loader_spec.rb @@ -38,16 +38,16 @@ describe Puppet::Parser::TypeLoader do it "should attempt to import each generated name" do @loader.expects(:name2files).returns %w{foo bar} - @loader.expects(:import).with("foo") - @loader.expects(:import).with("bar") + @loader.expects(:import).with("foo",nil) + @loader.expects(:import).with("bar",nil) @loader.load_until(["foo"], "bar") { |f| false } end it "should yield after each import" do yielded = [] @loader.expects(:name2files).returns %w{foo bar} - @loader.expects(:import).with("foo") - @loader.expects(:import).with("bar") + @loader.expects(:import).with("foo",nil) + @loader.expects(:import).with("bar",nil) @loader.load_until(["foo"], "bar") { |f| yielded << f; false } yielded.should == %w{foo bar} end @@ -55,31 +55,31 @@ describe Puppet::Parser::TypeLoader do it "should stop importing when the yielded block returns true" do yielded = [] @loader.expects(:name2files).returns %w{foo bar baz} - @loader.expects(:import).with("foo") - @loader.expects(:import).with("bar") - @loader.expects(:import).with("baz").never + @loader.expects(:import).with("foo",nil) + @loader.expects(:import).with("bar",nil) + @loader.expects(:import).with("baz",nil).never @loader.load_until(["foo"], "bar") { |f| true if f == "bar" } end it "should return the result of the block" do yielded = [] @loader.expects(:name2files).returns %w{foo bar baz} - @loader.expects(:import).with("foo") - @loader.expects(:import).with("bar") - @loader.expects(:import).with("baz").never + @loader.expects(:import).with("foo",nil) + @loader.expects(:import).with("bar",nil) + @loader.expects(:import).with("baz",nil).never @loader.load_until(["foo"], "bar") { |f| 10 if f == "bar" }.should == 10 end it "should return nil if the block never returns true" do @loader.expects(:name2files).returns %w{foo bar} - @loader.expects(:import).with("foo") - @loader.expects(:import).with("bar") + @loader.expects(:import).with("foo",nil) + @loader.expects(:import).with("bar",nil) @loader.load_until(["foo"], "bar") { |f| false }.should be_nil end it "should know when a given name has been loaded" do @loader.expects(:name2files).returns %w{file} - @loader.expects(:import).with("file") + @loader.expects(:import).with("file",nil) @loader.load_until(["foo"], "bar") { |f| true } @loader.should be_loaded("file") end -- 1.6.4 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
