Fix possible memory leak in use of overrides of BlockFile::CalcSummary

src/AudacityApp.cpp

@@ -1977,7 +1977,6 @@ int AudacityApp::OnExit()
 #endif
 
    DeinitFFT();
-   BlockFile::Deinit();
 
    DeinitAudioIO();
 

src/BlockFile.cpp

@@ -86,7 +86,7 @@ SummaryInfo::SummaryInfo(sampleCount samples)
    totalSummaryBytes = offset256 + (frames256 * bytesPerFrame);
 }
 
-char *BlockFile::fullSummary = 0;
+ArrayOf<char> BlockFile::fullSummary;
 
 /// Initializes the base BlockFile data.  The block is initially
 /// unlocked and its reference count is 1.
@@ -181,11 +181,6 @@ bool BlockFile::Deref()
       return false;
 }
 
-void BlockFile::Deinit()
-{
-   if(fullSummary)delete[] fullSummary;
-}
-
 /// Get a buffer containing a summary block describing this sample
 /// data.  This must be called by derived classes when they
 /// are constructed, to allow them to construct their summary data,
@@ -201,15 +196,17 @@ void BlockFile::Deinit()
 /// @param len    The length of the sample data
 /// @param format The format of the sample data.
 void *BlockFile::CalcSummary(samplePtr buffer, sampleCount len,
-                             sampleFormat format)
+                             sampleFormat format, ArrayOf<char> &cleanup)
 {
-   if(fullSummary)delete[] fullSummary;
-   fullSummary = new char[mSummaryInfo.totalSummaryBytes];
+   // Caller has nothing to deallocate
+   cleanup.reset();
 
-   memcpy(fullSummary, headerTag, headerTagLen);
+   fullSummary.reinit(mSummaryInfo.totalSummaryBytes);
 
-   float *summary64K = (float *)(fullSummary + mSummaryInfo.offset64K);
-   float *summary256 = (float *)(fullSummary + mSummaryInfo.offset256);
+   memcpy(fullSummary.get(), headerTag, headerTagLen);
+
+   float *summary64K = (float *)(fullSummary.get() + mSummaryInfo.offset64K);
+   float *summary256 = (float *)(fullSummary.get() + mSummaryInfo.offset256);
 
    float *fbuffer = new float[len];
    CopySamples(buffer, format,
@@ -303,7 +300,7 @@ void *BlockFile::CalcSummary(samplePtr buffer, sampleCount len,
 
    delete[] fbuffer;
 
-   return fullSummary;
+   return fullSummary.get();
 }
 
 static void ComputeMinMax256(float *summary256,
@@ -581,8 +578,9 @@ void AliasBlockFile::WriteSummary()
    SampleBuffer sampleData(mLen, floatSample);
    this->ReadData(sampleData.ptr(), floatSample, 0, mLen);
 
+   ArrayOf<char> cleanup;
    void *summaryData = BlockFile::CalcSummary(sampleData.ptr(), mLen,
-                                            floatSample);
+                                            floatSample, cleanup);
    summaryFile.Write(summaryData, mSummaryInfo.totalSummaryBytes);
 }
 

src/BlockFile.h

@@ -12,6 +12,7 @@
 #ifndef __AUDACITY_BLOCKFILE__
 #define __AUDACITY_BLOCKFILE__
 
+#include "MemoryX.h"
 #include <wx/string.h>
 #include <wx/ffile.h>
 #include <wx/filename.h>
@@ -47,8 +48,6 @@ class PROFILE_DLL_API BlockFile /* not final, abstract */ {
    BlockFile(wxFileName fileName, sampleCount samples);
    virtual ~BlockFile();
 
-   static void Deinit();
-
    // Reading
 
    /// Retrieves audio data from this BlockFile
@@ -147,7 +146,9 @@ class PROFILE_DLL_API BlockFile /* not final, abstract */ {
  protected:
    /// Calculate summary data for the given sample data
    virtual void *CalcSummary(samplePtr buffer, sampleCount len,
-                             sampleFormat format);
+                             sampleFormat format,
+                             // This gets filled, if the caller needs to deallocate.  Else it is null.
+                             ArrayOf<char> &cleanup);
    /// Read the summary section of the file.  Derived classes implement.
    virtual bool ReadSummary(void *data) = 0;
 
@@ -159,7 +160,7 @@ class PROFILE_DLL_API BlockFile /* not final, abstract */ {
    int mLockCount;
    int mRefCount;
 
-   static char *fullSummary;
+   static ArrayOf<char> fullSummary;
 
  protected:
    wxFileName mFileName;

src/blockfile/ODDecodeBlockFile.cpp

@@ -337,14 +337,12 @@ int ODDecodeBlockFile::WriteODDecodeBlockFile()
          sampleData.ptr(),
          mLen,
          mFormat,
-         NULL);//summaryData);
+         NULL);
    wxASSERT(bSuccess); // TODO: Handle failure here by alert to user and undo partial op.
    wxUnusedVar(bSuccess);
 
    mFileNameMutex.Unlock();
 
-//   delete [] (char *) summaryData;
-
 
    mDataAvailableMutex.Lock();
    mDataAvailable=true;
@@ -394,9 +392,10 @@ wxFileName ODDecodeBlockFile::GetFileName()
 /// @param len    The length of the sample data
 /// @param format The format of the sample data.
 void *ODDecodeBlockFile::CalcSummary(samplePtr buffer, sampleCount len,
-                             sampleFormat format)
+                             sampleFormat format, ArrayOf<char> &cleanup)
 {
-   char* localFullSummary = new char[mSummaryInfo.totalSummaryBytes];
+   cleanup.reinit(mSummaryInfo.totalSummaryBytes);
+   char* localFullSummary = cleanup.get();
 
    memcpy(localFullSummary, bheaderTag, bheaderTagLen);
 

src/blockfile/ODDecodeBlockFile.h

@@ -156,7 +156,7 @@ class ODDecodeBlockFile final : public SimpleBlockFile
 
 //   void WriteSimpleBlockFile() override;
    void *CalcSummary(samplePtr buffer, sampleCount len,
-                             sampleFormat format) override;
+                             sampleFormat format, ArrayOf<char> &cleanup) override;
    //The on demand type.
    unsigned int mType;
 

src/blockfile/ODPCMAliasBlockFile.cpp

@@ -424,13 +424,13 @@ void ODPCMAliasBlockFile::WriteSummary()
    SampleBuffer sampleData(mLen, floatSample);
    this->ReadData(sampleData.ptr(), floatSample, 0, mLen);
 
+   ArrayOf<char> cleanup;
    void *summaryData = CalcSummary(sampleData.ptr(), mLen,
-                                            floatSample);
+                                            floatSample, cleanup);
 
    //summaryFile.Write(summaryData, mSummaryInfo.totalSummaryBytes);
    fwrite(summaryData, 1, mSummaryInfo.totalSummaryBytes, summaryFile);
    fclose(summaryFile);
-   delete [] (char *) summaryData;
 
 
     //     printf("write successful. filename: %s\n", fileNameChar);
@@ -460,9 +460,10 @@ void ODPCMAliasBlockFile::WriteSummary()
 /// @param len    The length of the sample data
 /// @param format The format of the sample data.
 void *ODPCMAliasBlockFile::CalcSummary(samplePtr buffer, sampleCount len,
-                             sampleFormat format)
+                             sampleFormat format, ArrayOf<char> &cleanup)
 {
-   char* localFullSummary = new char[mSummaryInfo.totalSummaryBytes];
+   cleanup.reinit(mSummaryInfo.totalSummaryBytes);
+   char* localFullSummary = cleanup.get();
 
    memcpy(localFullSummary, aheaderTag, aheaderTagLen);
 

src/blockfile/ODPCMAliasBlockFile.h

@@ -138,7 +138,7 @@ class ODPCMAliasBlockFile final : public PCMAliasBlockFile
 protected:
    void WriteSummary() override;
    void *CalcSummary(samplePtr buffer, sampleCount len,
-      sampleFormat format) override;
+      sampleFormat format, ArrayOf<char> &cleanup) override;
 
   private:
    //Thread-safe versions

src/blockfile/SimpleBlockFile.cpp

@@ -123,8 +123,9 @@ SimpleBlockFile::SimpleBlockFile(wxFileName baseFileName,
       mCache.sampleData = new char[sampleLen * SAMPLE_SIZE(format)];
       memcpy(mCache.sampleData,
              sampleData, sampleLen * SAMPLE_SIZE(format));
+      ArrayOf<char> cleanup;
       void* summaryData = BlockFile::CalcSummary(sampleData, sampleLen,
-                                                format);
+         format, cleanup);
       mCache.summaryData = new char[mSummaryInfo.totalSummaryBytes];
       memcpy(mCache.summaryData, summaryData,
              (size_t)mSummaryInfo.totalSummaryBytes);
@@ -207,8 +208,11 @@ bool SimpleBlockFile::WriteSimpleBlockFile(
    header.channels = 1;
 
    // Write the file
+   ArrayOf<char> cleanup;
    if (!summaryData)
-      summaryData = /*BlockFile::*/CalcSummary(sampleData, sampleLen, format); //mchinen:allowing virtual override of calc summary for ODDecodeBlockFile.
+      summaryData = /*BlockFile::*/CalcSummary(sampleData, sampleLen, format, cleanup);
+      //mchinen:allowing virtual override of calc summary for ODDecodeBlockFile.
+      // PRL: cleanup fixes a possible memory leak!
 
    size_t nBytesToWrite = sizeof(header);
    size_t nBytesWritten = file.Write(&header, nBytesToWrite);