* 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: MarkusQ <[email protected]>
---
lib/puppet/parser/parser_support.rb | 116 +++++++++++++++++++----------------
1 files changed, 64 insertions(+), 52 deletions(-)
diff --git a/lib/puppet/parser/parser_support.rb
b/lib/puppet/parser/parser_support.rb
index e1af2fe..224c2db 100644
--- a/lib/puppet/parser/parser_support.rb
+++ b/lib/puppet/parser/parser_support.rb
@@ -112,36 +112,22 @@ class Puppet::Parser::Parser
end
def find_or_load(namespace, name, type)
- method = "find_" + type.to_s
- if result = @loaded_code.send(method, namespace, name)
- return result
- end
-
+ method = "find_#{type}"
fullname = (namespace + "::" + name).sub(/^::/, '')
- self.load(fullname)
-
- if result = @loaded_code.send(method, namespace, name)
- return result
- end
-
- # Try to load the module init file if we're a qualified
- # name
- if fullname.include?("::")
- module_name = fullname.split("::")[0]
+ things_to_try = [fullname]
- self.load(module_name)
+ # Try to load the module init file if we're a qualified name
+ things_to_try << fullname.split("::")[0] if fullname.include?("::")
- if result = @loaded_code.send(method, namespace, name)
- return result
- end
- end
-
- # Now try to load the bare name on its own. This is
- # appropriate if the class we're looking for is in a
+ # Otherwise try to load the bare name on its own. This
+ # is appropriate if the class we're looking for is in a
# module that's different from our namespace.
- self.load(name)
+ things_to_try << name
- @loaded_code.send(method, namespace, name)
+ until (result = @loaded_code.send(method, namespace, name)) or
things_to_try.empty? do
+ self.load(things_to_try.shift)
+ end
+ return result
end
# Import our files.
@@ -207,42 +193,68 @@ class Puppet::Parser::Parser
@lexer = Puppet::Parser::Lexer.new()
@files = {}
@loaded = []
- end
-
- # Try to load a class, since we could not find it.
- def load(classname)
- return false if classname == ""
- filename = classname.gsub("::", File::SEPARATOR)
-
- # First try to load the top-level module
- mod = filename.scan(/^[\w-]+/).shift
- unless @loaded.include?(mod)
- @loaded << mod
- begin
- import(mod)
- Puppet.info "Autoloaded module %s" % mod
- rescue Puppet::ImportError => detail
- # We couldn't load the module
+ @loading = {}
+ @loading.extend(MonitorMixin)
+ class << @loading
+ def done_with(item)
+ synchronize do
+ delete(item)[:busy].signal if self.has_key?(item) and
self[item][:loader] == Thread.current
+ end
+ end
+ def owner_of(item)
+ synchronize do
+ if !self.has_key? item
+ self[item] = { :loader => Thread.current, :busy =>
self.new_cond}
+ :nobody
+ elsif self[item][:loader] == Thread.current
+ :this_thread
+ else
+ flag = self[item][:busy]
+ flag.wait
+ flag.signal
+ :another_thread
+ end
+ end
end
end
+ end
- # We don't know whether we're looking for a class or definition, so we
have
- # to test for both.
- return true if @loaded_code.hostclass(classname) ||
@loaded_code.definition(classname)
-
- unless @loaded.include?(filename)
- @loaded << filename
- # Then the individual file
+ # Utility method factored out of load
+ def able_to_import?(classname,item,msg)
+ unless @loaded.include?(item)
begin
- import(filename)
- Puppet.info "Autoloaded file %s from module %s" % [filename,
mod]
+ case @loading.owner_of(item)
+ when :this_thread
+ return
+ when :another_thread
+ return able_to_import?(classname,item,msg)
+ when :nobody
+ import(item)
+ Puppet.info "Autoloaded #{msg}"
+ @loaded << item
+ end
rescue Puppet::ImportError => detail
- # We couldn't load the file
+ # We couldn't load the item
+ ensure
+ @loading.done_with(item)
end
end
# We don't know whether we're looking for a class or definition, so we
have
# to test for both.
- return true if @loaded_code.hostclass(classname) ||
@loaded_code.definition(classname)
+ return @loaded_code.hostclass(classname) ||
@loaded_code.definition(classname)
+ end
+
+ # Try to load a class, since we could not find it.
+ def load(classname)
+ return false if classname == ""
+ filename = classname.gsub("::", File::SEPARATOR)
+ mod = filename.scan(/^[\w-]+/).shift
+
+ # First try to load the top-level module then the individual file
+ # First try to load the top-level module then the individual file
+ [[mod, "module %s" % mod ],
+ [filename,"file %s from module %s" % [filename, mod]]
+ ].any? { |item,description|
able_to_import?(classname,item,description) }
end
# Split an fq name into a namespace and name
--
1.6.0.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
-~----------~----~----~----~------~----~------~--~---
