Fixed issue #4570 (Race conditions when serializing objects to YAML).

The ZAML class was using class variables to keep track of labels and
backreferences while serializing object to YAML.  This made it
possible to get ill-formed or incorrect YAML output if two threads
tried to serialize objects at the same time.

Changed to use instance variables of the ZAML class, so there is no
race condition.

Also added some more spec tests to verify that labels are generated
properly.

2 files changed, 36 insertions, 12 deletions

diff --git a/lib/puppet/util/zaml.rb b/lib/puppet/util/zaml.rb
index 8ecc2c8bd..b60e639ff 100644
--- a/lib/puppet/util/zaml.rb
+++ b/lib/puppet/util/zaml.rb
@@ -29,7 +29,8 @@ class ZAML
     @result = []
     @indent = nil
     @structured_key_prefix = nil
-    Label.counter_reset
+    @previously_emitted_object = {}
+    @next_free_label_number = 0
     emit('--- ')
   end
   def nested(tail='  ')
@@ -55,31 +56,29 @@ class ZAML
     #    which we will encounter a reference to the object as we serialize
     #    it can be handled).
     #
-    def self.counter_reset
-      @@previously_emitted_object = {}
-      @@next_free_label_number = 0
-    end
+    attr_accessor :this_label_number
     def initialize(obj,indent)
       @indent = indent
       @this_label_number = nil
-      @@previously_emitted_object[obj.object_id] = self
     end
     def to_s
       @this_label_number ? ('&id%03d%s' % [@this_label_number, @indent]) : ''
     end
     def reference
-      @this_label_number ||= (@@next_free_label_number += 1)
       @reference         ||= '*id%03d' % @this_label_number
     end
-    def self.for(obj)
-      @@previously_emitted_object[obj.object_id]
-    end
+  end
+  def label_for(obj)
+    @previously_emitted_object[obj.object_id]
   end
   def new_label_for(obj)
-    Label.new(obj,(Hash === obj || Array === obj) ? "#{@indent || "\n"}  " : ' ')
+    label = Label.new(obj,(Hash === obj || Array === obj) ? "#{@indent || "\n"}  " : ' ')
+    @previously_emitted_object[obj.object_id] = label
+    label
   end
   def first_time_only(obj)
-    if label = Label.for(obj)
+    if label = label_for(obj)
+      label.this_label_number ||= (@next_free_label_number += 1)
       emit(label.reference)
     else
       if @structured_key_prefix and not obj.is_a? String
diff --git a/spec/unit/util/zaml_spec.rb b/spec/unit/util/zaml_spec.rb
index 4de57e6d3..14cf94f36 100644
--- a/spec/unit/util/zaml_spec.rb
+++ b/spec/unit/util/zaml_spec.rb
@@ -34,5 +34,30 @@ describe "Pure ruby yaml implementation" do
       lambda { YAML.load(o.to_yaml) }.should_not raise_error
     end
   }
+
+  it "should emit proper labels and backreferences for common objects" do
+    # Note: this test makes assumptions about the names ZAML chooses
+    # for labels.
+    x = [1, 2]
+    y = [3, 4]
+    z = [x, y, x, y]
+    z.to_yaml.should == "--- \n  - &id001\n    - 1\n    - 2\n  - &id002\n    - 3\n    - 4\n  - *id001\n  - *id002"
+    z2 = YAML.load(z.to_yaml)
+    z2.should == z
+    z2[0].should equal(z2[2])
+    z2[1].should equal(z2[3])
+  end
+
+  it "should emit proper labels and backreferences for recursive objects" do
+    x = [1, 2]
+    x << x
+    x.to_yaml.should == "--- &id001\n  \n  - 1\n  - 2\n  - *id001"
+    x2 = YAML.load(x.to_yaml)
+    x2.should be_a(Array)
+    x2.length.should == 3
+    x2[0].should == 1
+    x2[1].should == 2
+    x2[2].should equal(x2)
+  end
 end