During my current dealings with the FLAC library I think I discovered a memory 
leak. After an encoder stream has finish()'ed, I believe you are supposed to 
use it again by calling init(). However, when verification is enabled, the 
init() routine will create a new stream decoder (to verify the data) without 
deleting (or reusing) the existing one. A small program demonstrating this is 
pasted here:

 $ cat main.cc 

#include <cstdlib>
#include <string>
#include <iostream>
#include <FLAC++/encoder.h>

class FlacWriter : public FLAC::Encoder::File
{
 public:
  inline bool dataStart(std::string const &filename);
  inline bool dataUpdate(char const *const data, int len);
  inline bool dataFinish();
};

inline bool FlacWriter::dataStart(std::string const &filename)
{
  if (set_verify(true) && init(filename) == FLAC__STREAM_ENCODER_INIT_STATUS_OK)
    return true;
  return false;
}

inline bool FlacWriter::dataUpdate(char const *const data, int len)
{
  return true;  // handle data here and call process(_interleaved)
}

inline bool FlacWriter::dataFinish()
{
  if (!finish())
    return false;
  return true;
}

int main(int argc, char *argv[])
{
  int numencodes = argc > 1 ? atoi(argv[1]) : 2;
  if (!numencodes)
    numencodes = 2;

  FlacWriter fw;

  for (int i = 0; i < numencodes; ++i)
  {
    if (!fw.dataStart("filname.flac"))
    {
      std::cout << "Failed init!" << std::endl;
      return 1;
    }

    while (true) // normally read audio data here in blocks, and update 
FlacWriter
    {
      if (!fw.dataUpdate("audio data read from file", 26))
      {
        std::cout << "Failed update!" << std::endl;
        return 1;
      }
      break;
    }
        
    if (!fw.dataFinish())
    {
      std::cout << "Failed finish!" << std::endl;
      return 1;
    }
  }
  return 0;
}

As can be seen from valgrind output, when the encoder is not reused after 
finish(), no leaks occur:

 $ g++ -Wall -O3 main.cc -lFLAC++ 
 $ valgrind --leak-check=full ./a.out 1
==30494== Memcheck, a memory error detector
==30494== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==30494== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
==30494== Command: ./a.out 1
==30494== 
==30494== 
==30494== HEAP SUMMARY:
==30494==     in use at exit: 0 bytes in 0 blocks
==30494==   total heap usage: 28 allocs, 28 frees, 144,168 bytes allocated
==30494== 
==30494== All heap blocks were freed -- no leaks are possible
==30494== 
==30494== For counts of detected and suppressed errors, rerun with: -v
==30494== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 20 from 8)

However for each time init() is called again on the encoder, a constant amount 
of bytes are not deleted:

 $ valgrind --leak-check=full ./a.out 2
==30495== Memcheck, a memory error detector
==30495== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==30495== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
==30495== Command: ./a.out 2
==30495== 
==30495== 
==30495== HEAP SUMMARY:
==30495==     in use at exit: 6,772 bytes in 5 blocks
==30495==   total heap usage: 52 allocs, 47 frees, 275,712 bytes allocated
==30495== 
==30495== 6,772 (8 direct, 6,764 indirect) bytes in 1 blocks are definitely 
lost in loss record 5 of 5
==30495==    at 0x400522F: calloc (vg_replace_malloc.c:418)
==30495==    by 0x4068847: FLAC__stream_decoder_new (stream_decoder.c:284)
==30495==    by 0x4074911: init_stream_internal_ (stream_encoder.c:1007)
==30495==    by 0x4075A98: init_FILE_internal_ (stream_encoder.c:1221)
==30495==    by 0x401EE65: FLAC::Encoder::File::init(char const*) 
(stream_encoder.cpp:459)
==30495==    by 0x401E9FC: FLAC::Encoder::File::init(std::string const&) 
(stream_encoder.cpp:464)
==30495==    by 0x804A300: main (in /home/svandijk/programming/FLACLEAK/a.out)
==30495== 
==30495== LEAK SUMMARY:
==30495==    definitely lost: 8 bytes in 1 blocks
==30495==    indirectly lost: 6,764 bytes in 4 blocks
==30495==      possibly lost: 0 bytes in 0 blocks
==30495==    still reachable: 0 bytes in 0 blocks
==30495==         suppressed: 0 bytes in 0 blocks
==30495== 
==30495== For counts of detected and suppressed errors, rerun with: -v
==30495== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 20 from 8)

In this example I used FLAC::Encoder::File, but the same thing happens with 
FLAC::Encoder::Stream and, from reading the source, with the C API, as long as 
one does not call FLAC__stream_encoder_delete() and FLAC__stream_encoder_new() 
between finish()'ing and init()'ing a stream.

The fix will have to be in init_stream_internal_() in 
src/libFLAC/stream_encoder.c:1007 (in libFLAC-1.2.1), where it should checked 
whether encoder->private_->verify.decoder == NULL before calling 
FLAC__stream_decoder_new(). If a decoder is already set (from a previous call 
to init(), it should first be deleted or (preferably) reused. Alternatively, 
the verification decoder can be deleted when finish() is called on the encoder 
that contains it.

Workarounds are to not reuse finished streams (though explicitly allowed by the 
API), disable verification, or always use the C API and delete() and new() the 
encoder after calling finish().

Please correct me if wrong about any of these things.

Thanks,
Bas Timmer
_______________________________________________
Flac-dev mailing list
[email protected]
http://lists.xiph.org/mailman/listinfo/flac-dev

Reply via email to