Add memContextCallbackClear() to prevent double free() calls.

If an object free() method was called manually when a callback was set then the callback would call free() again.  This meant that each free() method had to protect against a subsequent call.

Instead, clear the callback (if present) before calling memContextFree().  This is faster (since there is no unecessary callback) and removes the need for semaphores to protect against a double free().

doc/xml/release.xml

@@ -90,6 +90,10 @@
                         <p>Change <code>infoArchiveCheckPg()</code> to display the <postgres/> version as a string (e.g. 9.4) instead of the integer representation (e.g. 90400) when throwing an error.</p>
                     </release-item>
 
+                    <release-item>
+                        <p>Add <code>memContextCallbackClear()</code> to prevent double <code>free()</code> calls.</p>
+                    </release-item>
+
                     <release-item>
                         <p>Merge <file>crypto/random</file> module into <file>crypto/crypto</file>.</p>
                     </release-item>

src/common/memContext.c

@@ -265,6 +265,32 @@ memContextCallback(MemContext *this, void (*callbackFunction)(void *), void *cal
     FUNCTION_TEST_RESULT_VOID();
 }
 
+/***********************************************************************************************************************************
+Clear the mem context callback.  This is usually done in the object free method after resources have been freed but before
+memContextFree() is called.  The goal is to prevent the object free method from being called more than once.
+***********************************************************************************************************************************/
+void
+memContextCallbackClear(MemContext *this)
+{
+    FUNCTION_TEST_BEGIN();
+        FUNCTION_TEST_PARAM(MEM_CONTEXT, this);
+
+        FUNCTION_TEST_ASSERT(this != NULL);
+    FUNCTION_TEST_END();
+
+    // Error if context is not active or freeing
+    ASSERT(this->state == memContextStateActive || this->state == memContextStateFreeing);
+
+    // Top context cannot have a callback
+    ASSERT(this != memContextTop());
+
+    // Clear callback function and argument
+    this->callbackFunction = NULL;
+    this->callbackArgument = NULL;
+
+    FUNCTION_TEST_RESULT_VOID();
+}
+
 /***********************************************************************************************************************************
 Allocate memory in the memory context and optionally zero it.
 ***********************************************************************************************************************************/

src/common/memContext.h

@@ -66,6 +66,7 @@ Use the MEM_CONTEXT*() macros when possible rather than implement error-handling
 MemContext *memContextNew(const char *name);
 void memContextMove(MemContext *this, MemContext *parentNew);
 void memContextCallback(MemContext *this, void (*callbackFunction)(void *), void *callbackArgument);
+void memContextCallbackClear(MemContext *this);
 MemContext *memContextSwitch(MemContext *this);
 void memContextFree(MemContext *this);
 

src/common/regExp.c

@@ -107,6 +107,8 @@ regExpFree(RegExp *this)
     if (this != NULL)
     {
         regfree(&this->regExp);
+
+        memContextCallbackClear(this->memContext);
         memContextFree(this->memContext);
     }
 

src/compress/gzipCompress.c

@@ -184,7 +184,7 @@ gzipCompressToLog(const GzipCompress *this)
 {
     return strNewFmt(
         "{inputSame: %s, done: %s, flushing: %s, availIn: %u}", cvtBoolToConstZ(this->inputSame), cvtBoolToConstZ(this->done),
-        cvtBoolToConstZ(this->done), this->stream != NULL ? this->stream->avail_in : 0);
+        cvtBoolToConstZ(this->done), this->stream->avail_in);
 }
 
 /***********************************************************************************************************************************
@@ -199,12 +199,10 @@ gzipCompressFree(GzipCompress *this)
 
     if (this != NULL)
     {
-        if (this->stream != NULL)
+        deflateEnd(this->stream);
-        {
+        this->stream = NULL;
-            deflateEnd(this->stream);
-            this->stream = NULL;
-        }
 
+        memContextCallbackClear(this->memContext);
         memContextFree(this->memContext);
     }
 

src/compress/gzipDecompress.c

@@ -159,7 +159,7 @@ gzipDecompressToLog(const GzipDecompress *this)
 {
     return strNewFmt(
         "{inputSame: %s, done: %s, availIn: %u}", cvtBoolToConstZ(this->inputSame), cvtBoolToConstZ(this->done),
-        this->stream != NULL ? this->stream->avail_in : 0);
+        this->stream->avail_in);
 }
 
 /***********************************************************************************************************************************
@@ -174,12 +174,9 @@ gzipDecompressFree(GzipDecompress *this)
 
     if (this != NULL)
     {
-        if (this->stream != NULL)
+        inflateEnd(this->stream);
-        {
-            inflateEnd(this->stream);
-            this->stream = NULL;
-        }
 
+        memContextCallbackClear(this->memContext);
         memContextFree(this->memContext);
     }
 

src/crypto/cipherBlock.c

@@ -291,6 +291,7 @@ cipherBlockFree(CipherBlock *this)
         EVP_CIPHER_CTX_cleanup(this->cipherContext);
 
     // Free mem context
+    memContextCallbackClear(this->memContext);
     memContextFree(this->memContext);
 
     FUNCTION_DEBUG_RESULT_VOID();

src/crypto/hash.c

@@ -219,12 +219,9 @@ cryptoHashFree(CryptoHash *this)
 
     if (this != NULL)
     {
-        if (this->hashContext != NULL)
+        EVP_MD_CTX_destroy(this->hashContext);
-        {
-            EVP_MD_CTX_destroy(this->hashContext);
-            this->hashContext = NULL;
-        }
 
+        memContextCallbackClear(this->memContext);
         memContextFree(this->memContext);
     }
 
@@ -295,7 +292,6 @@ cryptoHashOneStr(const String *type, String *message)
     FUNCTION_TEST_RESULT(BUFFER, cryptoHashOneC(type, (const unsigned char *)strPtr(message), strSize(message)));
 }
 
-
 /***********************************************************************************************************************************
 Get hmac for one message/key
 ***********************************************************************************************************************************/

src/storage/driver/posix/fileRead.c

@@ -250,6 +250,8 @@ storageDriverPosixFileReadFree(StorageDriverPosixFileRead *this)
     if (this != NULL)
     {
         storageDriverPosixFileReadClose(this);
+
+        memContextCallbackClear(this->memContext);
         memContextFree(this->memContext);
     }
 

src/storage/driver/posix/fileWrite.c

@@ -354,6 +354,8 @@ storageDriverPosixFileWriteFree(StorageDriverPosixFileWrite *this)
     if (this != NULL)
     {
         storageDriverPosixFileWriteClose(this);
+
+        memContextCallbackClear(this->memContext);
         memContextFree(this->memContext);
     }
 

test/src/module/common/memContextTest.c

@@ -232,6 +232,10 @@ testRun(void)
             memContextCallback(memContext, (MemContextCallback)testFree, memContext),
             AssertError, "callback is already set for context 'test-callback'");
 
+        // Clear and reset it
+        memContextCallbackClear(memContext);
+        memContextCallback(memContext, (MemContextCallback)testFree, memContext);
+
         memContextFree(memContext);
         TEST_RESULT_PTR(memContextCallbackArgument, memContext, "callback argument is context");
     }

test/src/module/compress/gzipTest.c

@@ -170,8 +170,6 @@ testRun(void)
 
         decompress->inputSame = true;
         decompress->done = true;
-        inflateEnd(decompress->stream);
-        decompress->stream = NULL;
         TEST_RESULT_STR(strPtr(gzipDecompressToLog(decompress)), "{inputSame: true, done: true, availIn: 0}", "format object");
     }