[Flac-dev] Memory leak

[Bastiaan Timmer]

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