Preparing to fix #1812 - Moving locking code to a module

This moves the locking code out of Puppet::Util into a
separate module, to make the code cleaner.

Signed-off-by: Luke Kanies <luke@madstop.com>

3 files changed, 53 insertions, 36 deletions

diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index a5f3c5b1a..f8a872122 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -10,6 +10,8 @@ module Puppet
 module Util
     require 'benchmark'
     
+    # These are all for backward compatibility -- these are methods that used
+    # to be in Puppet::Util but have been moved into external modules.
     require 'puppet/util/posix'
     extend Puppet::Util::POSIX
 
@@ -66,40 +68,6 @@ module Util
         end
     end
 
-    # Create a shared lock for reading
-    def self.readlock(file)
-        self.sync(file).synchronize(Sync::SH) do
-            File.open(file) { |f|
-                f.lock_shared { |lf| yield lf }
-            }
-        end
-    end
-
-    # Create an exclusive lock for writing, and do the writing in a
-    # tmp file.
-    def self.writelock(file, mode = 0600)
-        tmpfile = file + ".tmp"
-        unless FileTest.directory?(File.dirname(tmpfile))
-            raise Puppet::DevError, "Cannot create %s; directory %s does not exist" %
-                [file, File.dirname(file)]
-        end
-        self.sync(file).synchronize(Sync::EX) do
-            File.open(file, "w", mode) do |rf|
-                rf.lock_exclusive do |lrf|
-                    File.open(tmpfile, "w", mode) do |tf|
-                        yield tf
-                    end
-                    begin
-                        File.rename(tmpfile, file)
-                    rescue => detail
-                        Puppet.err "Could not rename %s to %s: %s" %
-                            [file, tmpfile, detail]
-                    end
-                end
-            end
-        end
-    end
-
     # Create instance methods for each of the log levels.  This allows
     # the messages to be a little richer.  Most classes will be calling this
     # method.
diff --git a/lib/puppet/util/file_locking.rb b/lib/puppet/util/file_locking.rb
new file mode 100644
index 000000000..80a0b2b0c
--- /dev/null
+++ b/lib/puppet/util/file_locking.rb
@@ -0,0 +1,47 @@
+require 'puppet/util'
+
+module Puppet::Util::FileLocking
+    module_function
+
+    # Create a shared lock for reading
+    def readlock(file)
+        Puppet::Util.sync(file).synchronize(Sync::SH) do
+            File.open(file) { |f|
+                f.lock_shared { |lf| yield lf }
+            }
+        end
+    end
+
+    # Create an exclusive lock for writing, and do the writing in a
+    # tmp file.
+    def writelock(file, mode = nil)
+        unless FileTest.directory?(File.dirname(file))
+            raise Puppet::DevError, "Cannot create %s; directory %s does not exist" % [file, File.dirname(file)]
+        end
+        tmpfile = file + ".tmp"
+
+        unless mode
+            begin
+                mode = File.stat(file).mode
+            rescue
+                mode = 0600
+            end
+        end
+
+        Puppet::Util.sync(file).synchronize(Sync::EX) do
+            File.open(file, "w", mode) do |rf|
+                rf.lock_exclusive do |lrf|
+                    File.open(tmpfile, "w", mode) do |tf|
+                        yield tf
+                    end
+                    begin
+                        File.rename(tmpfile, file)
+                    rescue => detail
+                        File.unlink(tmpfile) if File.exist?(tmpfile)
+                        raise Puppet::Error, "Could not rename %s to %s: %s; file %s was unchanged" % [file, tmpfile, Thread.current.object_id, detail, file]
+                    end
+                end
+            end
+        end
+    end
+end
diff --git a/lib/puppet/util/storage.rb b/lib/puppet/util/storage.rb
index dc4e9cd71..01c411181 100644
--- a/lib/puppet/util/storage.rb
+++ b/lib/puppet/util/storage.rb
@@ -1,6 +1,8 @@
 require 'yaml'
 require 'sync'
 
+require 'puppet/util/file_locking'
+
 # a class for storing state
 class Puppet::Util::Storage
     include Singleton
@@ -59,7 +61,7 @@ class Puppet::Util::Storage
             return
         end
         Puppet::Util.benchmark(:debug, "Loaded state") do
-            Puppet::Util.readlock(Puppet[:statefile]) do |file|
+            Puppet::Util::FileLocking.readlock(Puppet[:statefile]) do |file|
                 begin
                     @@state = YAML.load(file)
                 rescue => detail
@@ -97,7 +99,7 @@ class Puppet::Util::Storage
         end
 
         Puppet::Util.benchmark(:debug, "Stored state") do
-            Puppet::Util.writelock(Puppet[:statefile], 0660) do |file|
+            Puppet::Util::FileLocking.writelock(Puppet[:statefile], 0660) do |file|
                 file.print YAML.dump(@@state)
             end
         end