[Pkg-puppet-devel] [facter] 116/180: (FACT-482) Remove thread locking

Stig Sandbeck Mathisen ssm at debian.org
Mon Jun 30 15:06:38 UTC 2014


This is an automated email from the git hooks/post-receive script.

ssm pushed a commit to branch master
in repository facter.

commit 159d3a682f141a5183c933be5ef2cf763f59bb6c
Author: Adrien Thebo <git at somethingsinistral.net>
Date:   Mon Jun 2 15:05:41 2014 -0700

    (FACT-482) Remove thread locking
    
    Facter has sporadic instances where facts implement their own locking
    when resolving facts. Facter itself has never made any guarantees about
    thread safety so the existing locking is useless at best and broken at
    worst.
    
    When Mcollective resolves facts it acquires a threadlock before running
    to ensure that facts are generated in an atomic fashion. However the
    lock used is not scoped to the given critical region; it is global so
    `Thread.exclusive` cannot be reentered in a single thread:
    
        [3] pry(main)> Thread.exclusive do
        [3] pry(main)*   Thread.exclusive do
        [3] pry(main)*     puts "hi"
        [3] pry(main)*   end
        [3] pry(main)* end
        ThreadError: deadlock; recursive locking
        from <internal:prelude>:8:in `lock'
    
    Because of this the facter facts source could not be used on Windows
    because Mcollective and Facter would deadlock. This commit removes the
    locking from Facter and places the responsibility for thread safety on
    the code invoking Facter.
---
 lib/facter/processor.rb      | 26 ++++++++++++--------------
 lib/facter/util/processor.rb | 18 ++++++++----------
 2 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/lib/facter/processor.rb b/lib/facter/processor.rb
index d317e93..3bc3e52 100644
--- a/lib/facter/processor.rb
+++ b/lib/facter/processor.rb
@@ -104,21 +104,19 @@ end
 if Facter.value(:kernel) == "windows"
   processor_list = []
 
-  Thread::exclusive do
-    require 'facter/util/wmi'
-
-    # get each physical processor
-    Facter::Util::WMI.execquery("select * from Win32_Processor").each do |proc|
-      # not supported before 2008
-      if proc.respond_to?(:NumberOfLogicalProcessors)
-        processor_num = proc.NumberOfLogicalProcessors
-      else
-        processor_num = 1
-      end
+  require 'facter/util/wmi'
 
-      processor_num.times do |i|
-        processor_list << proc.Name.squeeze(" ")
-      end
+  # get each physical processor
+  Facter::Util::WMI.execquery("select * from Win32_Processor").each do |proc|
+    # not supported before 2008
+    if proc.respond_to?(:NumberOfLogicalProcessors)
+      processor_num = proc.NumberOfLogicalProcessors
+    else
+      processor_num = 1
+    end
+
+    processor_num.times do |i|
+      processor_list << proc.Name.squeeze(" ")
     end
   end
 
diff --git a/lib/facter/util/processor.rb b/lib/facter/util/processor.rb
index 05a5f98..76260d3 100644
--- a/lib/facter/util/processor.rb
+++ b/lib/facter/util/processor.rb
@@ -271,16 +271,14 @@ module Processor
   def self.enum_kstat
     processor_num = -1
     processor_list = []
-    Thread::exclusive do
-      kstat = Facter::Core::Execution.exec('/usr/bin/kstat cpu_info')
-      if kstat
-        kstat.each_line do |l|
-          if l =~ /cpu_info(\d+)/
-            processor_num = $1.to_i
-          elsif l =~ /brand\s+(.*)\s*$/
-            processor_list[processor_num] = $1 unless processor_num == -1
-            processor_num = -1
-          end
+    kstat = Facter::Core::Execution.exec('/usr/bin/kstat cpu_info')
+    if kstat
+      kstat.each_line do |l|
+        if l =~ /cpu_info(\d+)/
+          processor_num = $1.to_i
+        elsif l =~ /brand\s+(.*)\s*$/
+          processor_list[processor_num] = $1 unless processor_num == -1
+          processor_num = -1
         end
       end
     end

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-puppet/facter.git



More information about the Pkg-puppet-devel mailing list