Improving fix for #1175; tightening thread safety

The previous code maintained thread safety up to work-duplication (so that a
collision would, at worse, result in effective cache flushing and cause some
additional work to be done).  The preceding patch addressed the single thread
issue of environment specific functions; this patch brings the thread safety
up to the previous standard.

3 files changed, 14 insertions, 10 deletions

diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index 4363eea41..81f8f2cfe 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -42,15 +42,15 @@ class Puppet::Node::Environment
     end
 
     def self.current
-        @current || root
+        Thread.current[:environment] || root
     end
 
     def self.current=(env)
-        @current = new(env)
+        Thread.current[:environment] = new(env)
     end
 
     def self.root
-        @root ||= new(:'*root*') 
+        @root
     end
 
     # This is only used for testing.
@@ -128,4 +128,5 @@ class Puppet::Node::Environment
         end
     end
 
+    @root = new(:'*root*') 
 end
diff --git a/lib/puppet/parser/ast/function.rb b/lib/puppet/parser/ast/function.rb
index ba4977935..0984ed8ce 100644
--- a/lib/puppet/parser/ast/function.rb
+++ b/lib/puppet/parser/ast/function.rb
@@ -13,7 +13,7 @@ class Puppet::Parser::AST
         def evaluate(scope)
 
             # Make sure it's a defined function
-            unless @fname = Puppet::Parser::Functions.function(@name)
+            unless Puppet::Parser::Functions.function(@name)
                 raise Puppet::ParseError, "Unknown function %s" % @name
             end
 
@@ -34,8 +34,6 @@ class Puppet::Parser::AST
                 raise Puppet::DevError, "Invalid function type %s" % @ftype.inspect
             end
 
-
-
             # We don't need to evaluate the name, because it's plaintext
             args = @arguments.safeevaluate(scope)
 
diff --git a/lib/puppet/parser/functions.rb b/lib/puppet/parser/functions.rb
index e0973c19d..201121cb9 100644
--- a/lib/puppet/parser/functions.rb
+++ b/lib/puppet/parser/functions.rb
@@ -1,13 +1,14 @@
 require 'puppet/util/autoload'
 require 'puppet/parser/scope'
+require 'monitor'
 
 # A module for managing parser functions.  Each specified function
 # is added to a central module that then gets included into the Scope
 # class.
 module Puppet::Parser::Functions
 
-    @functions = Hash.new { |h,k| h[k] = {} }
-    @modules = {}
+    (@functions = Hash.new { |h,k| h[k] = {} }).extend(MonitorMixin)
+    (@modules   = {}                          ).extend(MonitorMixin)
 
     class << self
         include Puppet::Util
@@ -27,7 +28,9 @@ module Puppet::Parser::Functions
     Environment = Puppet::Node::Environment
 
     def self.environment_module(env = nil)
-        @modules[ env || Environment.current || Environment.root ] ||= Module.new
+        @modules.synchronize { 
+            @modules[ env || Environment.current || Environment.root ] ||= Module.new 
+        }
     end
 
     # Create a new function type.
@@ -101,7 +104,9 @@ module Puppet::Parser::Functions
     end
 
     def self.functions(env = nil)
-        @functions[ env || Environment.current || Environment.root ]
+        @functions.synchronize { 
+            @functions[ env || Environment.current || Environment.root ] 
+        }
     end
 
     # Determine if a given function returns a value or not.