Did some work on making sure object removal actually works, thus stopping some potential memory leaks. Also explicitly removed objects in more places, again hopefully forestalling memory leaks.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1224 980ebf18-57e1-0310-9a29-db15c13687c0

11 files changed, 71 insertions, 82 deletions

diff --git a/lib/puppet/client/dipper.rb b/lib/puppet/client/dipper.rb
index 7271eded9..a9de76e6f 100644
--- a/lib/puppet/client/dipper.rb
+++ b/lib/puppet/client/dipper.rb
@@ -26,13 +26,7 @@ module Puppet
                 unless FileTest.exists?(file)
                     raise(BucketError, "File %s does not exist" % file)
                 end
-                contents = File.read(file)
-                string = Base64.encode64(contents)
-
-                sum = @driver.addfile(string,file)
-                string = ""
-                contents = ""
-                return sum
+                return @driver.addfile(Base64.encode64(File.read(file)),file)
             end
 
             # Restore the file
diff --git a/lib/puppet/client/master.rb b/lib/puppet/client/master.rb
index 9194b86be..3f29eb44e 100644
--- a/lib/puppet/client/master.rb
+++ b/lib/puppet/client/master.rb
@@ -110,6 +110,12 @@ class Puppet::Client::MasterClient < Puppet::Client
         @cachefile
     end
 
+    def clear
+        #@objects = nil
+        @objects.remove(true)
+        Puppet::Type.allclear
+    end
+
     # Disable running the configuration.  This can be used from the command line, but
     # is also used to make sure only one client is running at a time.
     def disable(running = false)
@@ -261,6 +267,9 @@ class Puppet::Client::MasterClient < Puppet::Client
         # Clear all existing objects, so we can recreate our stack.
         if defined? @objects
             Puppet::Type.allclear
+
+            # Make sure all of the objects are really gone.
+            @objects.remove(true)
         end
         @objects = nil
 
@@ -273,7 +282,6 @@ class Puppet::Client::MasterClient < Puppet::Client
         if @objects.nil?
             raise Puppet::Error, "Configuration could not be processed"
         end
-        #@objects = objects
 
         # and perform any necessary final actions before we evaluate.
         @objects.finalize
diff --git a/lib/puppet/config.rb b/lib/puppet/config.rb
index e7ad95dd4..680c9f2fb 100644
--- a/lib/puppet/config.rb
+++ b/lib/puppet/config.rb
@@ -523,7 +523,9 @@ Generated on #{Time.now}.
 
             # Remove is a recursive process, so it's sufficient to just call
             # it on the component.
-            objects.remove
+            objects.remove(true)
+
+            objects = nil
 
             runners.each { |s| @used << s }
         end
diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb
index f88e412ec..102a9fa78 100644
--- a/lib/puppet/parameter.rb
+++ b/lib/puppet/parameter.rb
@@ -366,6 +366,10 @@ module Puppet
             end
         end
 
+        def remove
+            @parent = nil
+        end
+
         # This should only be called for parameters, but go ahead and make
         # it possible to call for states, too.
         def value
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 7e20f3731..95691aaee 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -36,13 +36,6 @@ class Type < Puppet::Element
 
     include Enumerable
 
-    # this is currently unused, but I expect to use it for metrics eventually
-    @@retrieved = Hash.new(0)
-
-    # an array to contain all instances of Type
-    # also currently unused
-    @@allobjects = Array.new
-
     # a little fakery, since Puppet itself isn't a type
     # I don't think this is used any more, now that the language can't
     # call methods
@@ -64,9 +57,9 @@ class Type < Puppet::Element
 
         include Enumerable
 
-        def inspect
-            "Type(%s)" % self.name
-        end
+        #def inspect
+        #    "Type(%s)" % self.name
+        #end
 
         # This class is aggregatable, meaning that instances are defined on
         # one system but instantiated on another
@@ -84,7 +77,7 @@ class Type < Puppet::Element
         end
     end
 
-    def inspect
+    def disabled_inspect
         str = "Type(%s)" % self.name
         if defined? @states
             str += " States(" + @states.inspect + ")"
@@ -122,19 +115,6 @@ class Type < Puppet::Element
         return ens
     end
 
-    # The work that gets done for every subclass of Type
-    # this is an implicit method called by Ruby for us
-    #def self.inherited(sub)
-    #    sub.initvars
-
-        #debug("subtype %s(%s) just created" % [sub,sub.superclass])
-        # add it to the master list
-        # unfortunately we can't yet call sub.name, because the #inherited
-        # method gets called before any commands in the class definition
-        # get executed, which, um, sucks
-        #@@typeary.push(sub)
-    #end
-
     # all of the variables that must be initialized for each subclass
     def self.initvars
         # all of the instances of this class
@@ -291,14 +271,10 @@ class Type < Puppet::Element
             #    [name,object.class])
             @objects[name] = newobj
         end
-
-        # and then add it to the master list
-        Puppet::Type.push(object)
     end
 
     # remove all type instances; this is mostly only useful for testing
     def self.allclear
-        @@allobjects.clear
         Puppet::Event::Subscription.clear
         @types.each { |name, type|
             type.clear
@@ -308,6 +284,9 @@ class Type < Puppet::Element
     # remove all of the instances of a single type
     def self.clear
         if defined? @objects
+            @objects.each do |name, obj|
+                obj.remove(true)
+            end
             @objects.clear
         end
         if defined? @aliases
@@ -317,9 +296,6 @@ class Type < Puppet::Element
 
     # remove a specified object
     def self.delete(object)
-        if @@allobjects.include?(object)
-            @@allobjects.delete(object)
-        end
         return unless defined? @objects
         if @objects.include?(object.name)
             @objects.delete(object.name)
@@ -357,14 +333,6 @@ class Type < Puppet::Element
         end
     end
 
-    # add an object to the master list of Type instances
-    # I'm pretty sure this is currently basically unused
-    def self.push(object)
-        @@allobjects.push object
-        #debug("adding %s of type %s to master list" %
-        #    [object.name,object.class])
-    end
-
     # class and instance methods dealing with parameters and states
 
     public
@@ -1032,6 +1000,18 @@ class Type < Puppet::Element
             child.remove(rmdeps)
         }
 
+        @children.clear
+
+        # This is hackish (mmm, cut and paste), but it works for now, and it's
+        # better than warnings.
+        [@states, @parameters, @metaparams].each do |hash|
+            hash.each do |name, obj|
+                obj.remove
+            end
+
+            hash.clear
+        end
+
         if rmdeps
             Puppet::Event::Subscription.dependencies(self).each { |dep|
                 #begin
@@ -1053,6 +1033,7 @@ class Type < Puppet::Element
 
         if defined? @parent and @parent
             @parent.delete(self)
+            @parent = nil
         end
     end
 
@@ -1779,7 +1760,6 @@ class Type < Puppet::Element
                 [self.name,self.class]
             @evalcount = 0
         end
-        @@retrieved[self] += 1
         @evalcount += 1
 
         changes = []
diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb
index 778b14b47..3a637f719 100644
--- a/lib/puppet/type/package.rb
+++ b/lib/puppet/type/package.rb
@@ -445,27 +445,6 @@ module Puppet
             end
         end
 
-        # Return a list of valid package types
-#        def self.getpkglist
-#            if @types.nil?
-#                if @default.nil?
-#                    self.init
-#                end
-#                @types = [@default]
-#            end
-#
-#            list = @types.collect { |type|
-#                if typeobj = Puppet::PackagingType[type]
-#                    # pull all of the objects
-#                    typeobj.list
-#                else
-#                    raise Puppet::Error, "Could not find package type '%s'" % type
-#                end
-#            }.flatten
-#            @listed = true
-#            return list
-#        end
-
         # Create a new package object from listed information
         def self.installedpkg(hash)
             unless hash.include? :type
diff --git a/lib/puppet/type/package/sun.rb b/lib/puppet/type/package/sun.rb
index b4b563461..c0c34a19d 100755
--- a/lib/puppet/type/package/sun.rb
+++ b/lib/puppet/type/package/sun.rb
@@ -7,7 +7,7 @@ module Puppet
                 "NAME" => nil,
                 "CATEGORY" => :category,
                 "ARCH" => :platform,
-                "VERSION" => :ensure,
+                "VERSION" => :version,
                 "BASEDIR" => :root,
                 "HOTLINE" => nil,
                 "EMAIL" => nil,
@@ -98,7 +98,7 @@ module Puppet
                 "NAME" => nil,
                 "CATEGORY" => :category,
                 "ARCH" => :platform,
-                "VERSION" => :ensure,
+                "VERSION" => :version,
                 "BASEDIR" => :root,
                 "HOTLINE" => nil,
                 "EMAIL" => nil,
@@ -117,6 +117,16 @@ module Puppet
                 process.each { |line|
                     case line
                     when /^$/:
+                        if self.is_a? Puppet::Type and type = self[:type]
+                            hash[:type] = type
+                        elsif self.is_a? Module and self.respond_to? :name
+                            hash[:type] = self.name
+                        else
+                            raise Puppet::DevError, "Cannot determine package type"
+                        end
+
+                        hash[:ensure] = :present
+
                         packages.push Puppet.type(:package).installedpkg(hash)
                         hash.clear
                     when /\s*(\w+):\s+(.+)/:
diff --git a/test/language/snippets.rb b/test/language/snippets.rb
index 0ba2fb6ed..f45a5799d 100755
--- a/test/language/snippets.rb
+++ b/test/language/snippets.rb
@@ -25,6 +25,7 @@ require 'puppettest'
 
 class TestSnippets < Test::Unit::TestCase
 	include TestPuppet
+    include ObjectSpace
     $snippetbase = File.join($puppetbase, "examples", "code", "snippets")
     
     def file2ast(file)
@@ -186,7 +187,7 @@ class TestSnippets < Test::Unit::TestCase
         assert_nothing_raised {
             trans.rollback
         }
-        assert(! FileTest.exists?(file))
+        assert(! FileTest.exists?(file), "%s still exists" % file)
     end
 
     def snippet_simpleselector(trans)
@@ -206,7 +207,7 @@ class TestSnippets < Test::Unit::TestCase
             trans.rollback
         }
         files.each { |file|
-            assert(! FileTest.exists?(file))
+            assert(! FileTest.exists?(file), "%s still exists" % file)
         }
     end
 
@@ -231,7 +232,7 @@ class TestSnippets < Test::Unit::TestCase
         assert_nothing_raised {
             trans.rollback
         }
-        assert(! FileTest.exists?(file))
+        assert(! FileTest.exists?(file), "%s still exists" % file)
     end
 
     def snippet_argumentdefaults(trans)
@@ -523,16 +524,17 @@ class TestSnippets < Test::Unit::TestCase
                         assert(obj.name)
 
                         if obj.is_a?(Puppet.type(:file))
-                            @@tmpfiles << obj.name
+                            @@tmpfiles << obj[:path]
                         end
                     }
                 }
                 assert_nothing_raised {
                     self.send(mname, trans)
                 }
+
+                client.clear
             }
             mname = mname.intern
-            #eval("alias %s %s" % [testname, mname])
         end
     }
 end
diff --git a/test/puppettest.rb b/test/puppettest.rb
index ce1a2918a..c65c95f49 100644
--- a/test/puppettest.rb
+++ b/test/puppettest.rb
@@ -7,6 +7,12 @@ require 'puppet'
 require 'test/unit'
 
 module TestPuppet
+    include ObjectSpace
+
+    def gcdebug(type)
+        Puppet.warning "%s: %s" % [type, ObjectSpace.each_object(type) { |o| }]
+    end
+
     def newcomp(*ary)
         name = nil
         if ary[0].is_a?(String)
@@ -300,7 +306,6 @@ module TestPuppet
 
     # If there are any fake data files, retrieve them
     def fakedata(dir)
-
         ary = [$puppetbase, "test"]
         ary += dir.split("/")
         dir = File.join(ary)
diff --git a/test/server/bucket.rb b/test/server/bucket.rb
index e9e13797d..3169781f8 100644
--- a/test/server/bucket.rb
+++ b/test/server/bucket.rb
@@ -22,9 +22,11 @@ class TestBucket < Test::Unit::TestCase
             @num = 1
         end
 
-        Puppet.err "#{Process.pid}: %s: %s" % [@num, memory()]
+        #Puppet.err "#{Process.pid}: %s: %s" % [@num, memory()]
         GC.start
+        #gcdebug(String)
     end
+
     # run through all of the files and exercise the filebucket methods
     def checkfiles(client)
         files = filelist()
@@ -66,6 +68,9 @@ class TestBucket < Test::Unit::TestCase
             assert(tsum == osum)
 
             # modify our tmp file
+            unless FileTest.writable?(tmppath)
+                File.chmod(0644, tmppath)
+            end
             File.open(tmppath,File::WRONLY|File::TRUNC) { |wf|
                 wf.print "This is some test text\n"
             }
@@ -109,7 +114,7 @@ class TestBucket < Test::Unit::TestCase
         end
 
         %w{
-            who bash vim sh uname /etc/passwd /etc/syslog.conf /etc/hosts 
+            who bash sh uname /etc/passwd /etc/syslog.conf /etc/hosts 
         }.each { |file|
             # if it's fully qualified, just add it
             if file =~ /^\//
@@ -234,7 +239,7 @@ class TestBucket < Test::Unit::TestCase
         unless pid
             raise "Uh, we don't have a child pid"
         end
-        system("kill %s" % pid)
+        Process.kill("TERM", pid)
     end
 end
 
diff --git a/test/server/pelement.rb b/test/server/pelement.rb
index 1a6458768..7bff1cb26 100644
--- a/test/server/pelement.rb
+++ b/test/server/pelement.rb
@@ -196,7 +196,7 @@ class TestPElementServer < Test::Unit::TestCase
                 Puppet.warning "%s does not respond to :list" % type.name
                 next
             end
-            #next unless type.name == :port
+            next unless type.name == :package
             Puppet.info "Describing each %s" % type.name
 
             # First do a listing from the server