(PUP-11373) Always lock the environment before its type collection

Puppetserver in MT mode can deadlock if one thread calls into the loaders,
which locks the environment, and then tries to lock the type_collection:

    - waiting to lock <3f27e688> (a org.jruby.gen.RubyObject0) owned by "qtp346358872-131" t@131
    /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/resource/type_collection.rb:188
    /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/resource/type_collection.rb:152
    /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/resource/type_collection.rb:152
    /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/loader/runtime3_type_loader.rb:79
    ...
    /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/concurrent/lock.rb:10
    /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/loader/loader.rb:153

While another thread locks the type_collection and tries to lock the environment:

    - waiting to lock <bd0f663> (a org.jruby.gen.RubyObject0) owned by "qtp346358872-67" t@76
    /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/node/environment.rb:248
    /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/parser/type_loader.rb:131
    ...
    /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/resource/type_collection.rb:203
    /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/concurrent/lock.rb:10
    /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/resource/type_collection.rb:188

This commit orders locking so we always acquire the environment lock before its
type collection lock.

Author: joshcooper

lib/puppet/resource/type_collection.rb

@@ -24,6 +24,7 @@ def initialize(env)
     @definitions = {}
     @nodes = {}
     @notfound = {}
+    # always lock the environment before acquiring this lock
     @lock = Puppet::Concurrent::Lock.new
 
     # So we can keep a list and match the first-defined regex
@@ -185,26 +186,29 @@ def version
   # Resolve namespaces and find the given object.  Autoload it if
   # necessary.
   def find_or_load(name, type)
-    @lock.synchronize do
-      # Name is always absolute, but may start with :: which must be removed
-      fqname = (name[0,2] == COLON_COLON ? name[2..-1] : name)
-
-      result = send(type, fqname)
-      unless result
-        if @notfound[ fqname ] && Puppet[ :ignoremissingtypes ]
-          # do not try to autoload if we already tried and it wasn't conclusive
-          # as this is a time consuming operation. Warn the user.
-          # Check first if debugging is on since the call to debug_once is expensive
-          if Puppet[:debug]
-            debug_once _("Not attempting to load %{type} %{fqname} as this object was missing during a prior compilation") % { type: type, fqname: fqname }
+    # always lock the environment before locking the type collection
+    @environment.lock.synchronize do
+      @lock.synchronize do
+        # Name is always absolute, but may start with :: which must be removed
+        fqname = (name[0,2] == COLON_COLON ? name[2..-1] : name)
+
+        result = send(type, fqname)
+        unless result
+          if @notfound[ fqname ] && Puppet[ :ignoremissingtypes ]
+            # do not try to autoload if we already tried and it wasn't conclusive
+            # as this is a time consuming operation. Warn the user.
+            # Check first if debugging is on since the call to debug_once is expensive
+            if Puppet[:debug]
+              debug_once _("Not attempting to load %{type} %{fqname} as this object was missing during a prior compilation") % { type: type, fqname: fqname }
+            end
+          else
+            fqname = munge_name(fqname)
+            result = loader.try_load_fqname(type, fqname)
+            @notfound[ fqname ] = result.nil?
           end
-        else
-          fqname = munge_name(fqname)
-          result = loader.try_load_fqname(type, fqname)
-          @notfound[ fqname ] = result.nil?
         end
+        result
       end
-      result
     end
   end