Fix Flac picture memory leak

Author: jameswyper

ext/taglib_flac/taglib_flac.i

@@ -86,6 +86,13 @@
       SWIG_RubyRemoveTracking(properties);
     }
 
+    TagLib::List<TagLib::FLAC::Picture *> list = file->pictureList();
+      for (TagLib::List<TagLib::FLAC::Picture *>::ConstIterator it = list.begin(); it != list.end(); it++) {
+        TagLib::FLAC::Picture *picture = (*it);
+        SWIG_RubyUnlinkObjects(picture);
+        SWIG_RubyRemoveTracking(picture);
+      }
+
     SWIG_RubyUnlinkObjects(ptr);
     SWIG_RubyRemoveTracking(ptr);
 

ext/taglib_flac/taglib_flac_wrap.cxx

@@ -2210,6 +2210,13 @@ SWIGINTERN void TagLib_FLAC_File_close(TagLib::FLAC::File *self){
       SWIG_RubyRemoveTracking(properties);
     }
 
+    TagLib::List<TagLib::FLAC::Picture *> list = file->pictureList();
+      for (TagLib::List<TagLib::FLAC::Picture *>::ConstIterator it = list.begin(); it != list.end(); it++) {
+        TagLib::FLAC::Picture *picture = (*it);
+        SWIG_RubyUnlinkObjects(picture);
+        SWIG_RubyRemoveTracking(picture);
+      }
+
     SWIG_RubyUnlinkObjects(ptr);
     SWIG_RubyRemoveTracking(ptr);
 

test/data/flac-create.cpp

@@ -10,8 +10,12 @@
 using namespace TagLib;
 
 int main(int argc, char **argv) {
-  if (argc != 2) {
-    std::cout << "usage: " << argv[0] << " file" << std::endl;
+  if ( 
+      ((argc < 2) || (argc > 3)) 
+    || 
+      ((argc == 3) && (argv[2] != "nopic")) 
+    ) {
+    std::cout << "usage: " << argv[0] << " file [nopic]" << std::endl;
     exit(1);
   }
   char *filename = argv[1];
@@ -39,20 +43,23 @@ int main(int argc, char **argv) {
   tag->addField("MULTIPLE", "A");
   tag->addField("MULTIPLE", "B", false);
 
-  FLAC::Picture *picture = new FLAC::Picture();
 
-  picture->setType(FLAC::Picture::FrontCover);
-  picture->setMimeType("image/jpeg");
-  picture->setDescription("Globe");
-  picture->setWidth(90);
-  picture->setHeight(90);
-  picture->setColorDepth(8);
-  picture->setNumColors(0);
+  if (argc == 2) {
+    FLAC::Picture *picture = new FLAC::Picture();
 
-  ByteVector data = getPictureData("globe_east_90.jpg");
-  picture->setData(data);
+    picture->setType(FLAC::Picture::FrontCover);
+    picture->setMimeType("image/jpeg");
+    picture->setDescription("Globe");
+    picture->setWidth(90);
+    picture->setHeight(90);
+    picture->setColorDepth(8);
+    picture->setNumColors(0);
 
-  file.addPicture(picture);
+    ByteVector data = getPictureData("globe_east_90.jpg");
+    picture->setData(data);
+
+    file.addPicture(picture);
+  }
 
   file.save();
 }

test/data/flac_nopic.flac

@@ -0,0 +1,36 @@
+===============================================================================
+flac - Command-line FLAC encoder/decoder version 1.3.2
+Copyright (C) 2000-2009  Josh Coalson
+Copyright (C) 2011-2016  Xiph.Org Foundation
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License
+as published by the Free Software Foundation; either version 2
+of the License, or (at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License along
+with this program; if not, write to the Free Software Foundation, Inc.,
+51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+===============================================================================
+
+This is the short help; for all options use 'flac --help'; for even more
+instructions use 'flac --explain'
+
+Be sure to read the list of known bugs at:
+http://xiph.org/flac/documentation_bugs.html
+
+To encode:
+  flac [-#] [INPUTFILE [...]]
+
+  -# is -0 (fastest compression) to -8 (highest compression); -5 is the default
+
+To decode:
+  flac -d [INPUTFILE [...]]
+
+To test:
+  flac -t [INPUTFILE [...]]

test/flac_picture_memory_test.rb

@@ -0,0 +1,43 @@
+require File.join(File.dirname(__FILE__), 'helper')
+
+class TestFlacPictureMemory < Test::Unit::TestCase
+
+  N = 10000
+
+  context "TagLib::FLAC::Picture" do
+ 
+    setup do
+      
+    end
+
+    should "release memory when closing flac file with picture data" do
+      c = 0
+      N.times do
+        TagLib::FLAC::File.open("test/data/flac.flac", false) do |f|
+          f.picture_list.each do |p|
+            x = p.data
+            c = c + 1
+          end
+        end
+      end
+      assert_equal N,c
+    end
+
+    should "process a flac file without picture data" do
+      c = 0
+      N.times do
+        TagLib::FLAC::File.open("test/data/flac_nopic.flac", false) do |f|
+          f.picture_list.each do |p|
+            x = p.data
+            c = c + 1
+          end
+        end
+      end
+      assert_equal 0,c
+    end
+
+    teardown do
+
+    end
+  end
+end