Skip to content
Permalink
Browse files

Various MemContext callback improvements.

Rename memContextCallback() to memContextCallbackSet() to be more consistent with other parts of the code.

Free all context memory when an exception is thrown from a callback.  Previously only the child contexts would be freed and this resulted in some allocations being lost.  In practice this is probably not a big deal since the process will likely terminate shortly, but there may well be cases where that is not true.
  • Loading branch information...
dwsteele committed May 3, 2019
1 parent 4a20d44 commit 7ae96949f18af90a9b07f1dd712f939ca7a1ec41
@@ -67,6 +67,10 @@
<p>Add <file>common/macro.h</file> for general-purpose macros.</p>
</release-item>

<release-item>
<p>Various <code>MemContext</code> callback improvements.</p>
</release-item>

<release-item>
<p>Various <code>Buffer</code> improvements.</p>
</release-item>
@@ -193,7 +193,7 @@ gzipCompressNew(int level, bool raw)
gzipError(deflateInit2(driver->stream, level, Z_DEFLATED, gzipWindowBits(raw), MEM_LEVEL, Z_DEFAULT_STRATEGY));

// Set free callback to ensure gzip context is freed
memContextCallback(driver->memContext, (MemContextCallback)gzipCompressFree, driver);
memContextCallbackSet(driver->memContext, (MemContextCallback)gzipCompressFree, driver);

// Create filter interface
this = ioFilterNewP(
@@ -169,7 +169,7 @@ gzipDecompressNew(bool raw)
gzipError(driver->result = inflateInit2(driver->stream, gzipWindowBits(raw)));

// Set free callback to ensure gzip context is freed
memContextCallback(driver->memContext, (MemContextCallback)gzipDecompressFree, driver);
memContextCallbackSet(driver->memContext, (MemContextCallback)gzipDecompressFree, driver);

// Create filter interface
this = ioFilterNewP(
@@ -200,7 +200,7 @@ cipherBlockProcessBlock(CipherBlock *this, const unsigned char *source, size_t s
cryptoError(!(this->cipherContext = EVP_CIPHER_CTX_new()), "unable to create context");

// Set free callback to ensure cipher context is freed
memContextCallback(this->memContext, (MemContextCallback)cipherBlockFree, this);
memContextCallbackSet(this->memContext, (MemContextCallback)cipherBlockFree, this);

// Initialize cipher
cryptoError(
@@ -161,7 +161,7 @@ cryptoHashNew(const String *type)
cryptoError((driver->hashContext = EVP_MD_CTX_create()) == NULL, "unable to create hash context");

// Set free callback to ensure hash context is freed
memContextCallback(driver->memContext, (MemContextCallback)cryptoHashFreeCallback, driver);
memContextCallbackSet(driver->memContext, (MemContextCallback)cryptoHashFreeCallback, driver);

// Initialize context
cryptoError(!EVP_DigestInit_ex(driver->hashContext, driver->hashType, NULL), "unable to initialize hash context");
@@ -331,7 +331,7 @@ execOpen(Exec *this)
ioWriteOpen(this->ioWriteExec);

// Set a callback so the handles will get freed
memContextCallback(this->memContext, (MemContextCallback)execFree, this);
memContextCallbackSet(this->memContext, (MemContextCallback)execFree, this);

FUNCTION_LOG_RETURN_VOID();
}
@@ -94,7 +94,7 @@ tlsClientNew(
this->context = SSL_CTX_new(method);
cryptoError(this->context == NULL, "unable to create TLS context");

memContextCallback(this->memContext, (MemContextCallback)tlsClientFree, this);
memContextCallbackSet(this->memContext, (MemContextCallback)tlsClientFree, this);

// Exclude SSL versions to only allow TLS and also disable compression
SSL_CTX_set_options(this->context, (long)(SSL_OP_ALL | SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | SSL_OP_NO_COMPRESSION));
@@ -240,7 +240,7 @@ memContextNew(const char *name)
Register a callback to be called just before the context is freed
***********************************************************************************************************************************/
void
memContextCallback(MemContext *this, void (*callbackFunction)(void *), void *callbackArgument)
memContextCallbackSet(MemContext *this, void (*callbackFunction)(void *), void *callbackArgument)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM(MEM_CONTEXT, this);
@@ -595,8 +595,20 @@ memContextFree(MemContext *this)
this->state = memContextStateFreeing;

// Execute callback if defined
bool rethrow = false;

if (this->callbackFunction)
this->callbackFunction(this->callbackArgument);
{
TRY_BEGIN()
{
this->callbackFunction(this->callbackArgument);
}
CATCH_ANY()
{
rethrow = true;
}
TRY_END();
}

// Free child context allocations
if (this->contextChildListSize > 0)
@@ -634,6 +646,10 @@ memContextFree(MemContext *this)
// Else reset the memory context so it can be reused
else
memset(this, 0, sizeof(MemContext));

// Rethrow the error that was caught in the callback
if (rethrow)
RETHROW();
}

FUNCTION_TEST_RETURN_VOID();
@@ -29,7 +29,7 @@ Space is reserved for this many allocations when a context is created. When mor
#define MEM_CONTEXT_ALLOC_INITIAL_SIZE 4

/***********************************************************************************************************************************
Memory context callback function type, useful for casts in memContextCallback()
Memory context callback function type, useful for casts in memContextCallbackSet()
***********************************************************************************************************************************/
typedef void (*MemContextCallback)(void *callbackArgument);

@@ -60,7 +60,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 memContextCallbackSet(MemContext *this, void (*callbackFunction)(void *), void *callbackArgument);
void memContextCallbackClear(MemContext *this);
MemContext *memContextSwitch(MemContext *this);
void memContextFree(MemContext *this);
@@ -65,7 +65,7 @@ regExpNew(const String *expression)
}

// Set free callback to ensure cipher context is freed
memContextCallback(this->memContext, (MemContextCallback)regExpFree, this);
memContextCallbackSet(this->memContext, (MemContextCallback)regExpFree, this);
}
MEM_CONTEXT_NEW_END();

@@ -402,7 +402,7 @@ xmlDocumentNew(const String *rootName)
this->xml = xmlNewDoc(BAD_CAST "1.0");

// Set callback to ensure xml document is freed
memContextCallback(this->memContext, (MemContextCallback)xmlDocumentFree, this);
memContextCallbackSet(this->memContext, (MemContextCallback)xmlDocumentFree, this);

this->root = xmlNodeNew(xmlNewNode(NULL, BAD_CAST strPtr(rootName)));
xmlDocSetRootElement(this->xml, this->root->node);
@@ -440,7 +440,7 @@ xmlDocumentNewC(const unsigned char *buffer, size_t bufferSize)
THROW_FMT(FormatError, "invalid xml");

// Set callback to ensure xml document is freed
memContextCallback(this->memContext, (MemContextCallback)xmlDocumentFree, this);
memContextCallbackSet(this->memContext, (MemContextCallback)xmlDocumentFree, this);

// Get the root node
this->root = xmlNodeNew(xmlDocGetRootElement(this->xml));
@@ -110,7 +110,7 @@ protocolClientNew(const String *name, const String *service, IoRead *read, IoWri
protocolClientNoOp(this);

// Set a callback to shutdown the protocol
memContextCallback(this->memContext, (MemContextCallback)protocolClientFree, this);
memContextCallbackSet(this->memContext, (MemContextCallback)protocolClientFree, this);
}
MEM_CONTEXT_NEW_END();

@@ -107,7 +107,7 @@ storageReadPosixOpen(THIS_VOID)
// On success set free callback to ensure file handle is freed
if (this->handle != -1)
{
memContextCallback(this->memContext, (MemContextCallback)storageReadPosixFree, this);
memContextCallbackSet(this->memContext, (MemContextCallback)storageReadPosixFree, this);
result = true;
}

@@ -162,7 +162,7 @@ storageWritePosixOpen(THIS_VOID)
}

// Set free callback to ensure file handle is freed
memContextCallback(this->memContext, (MemContextCallback)storageWritePosixFree, this);
memContextCallbackSet(this->memContext, (MemContextCallback)storageWritePosixFree, this);

// Update user/group owner
if (this->interface.user != NULL || this->interface.group != NULL)
@@ -119,7 +119,7 @@ storageWriteRemoteOpen(THIS_VOID)
protocolClientExecute(this->client, command, false);

// Set free callback to ensure remote file is freed
memContextCallback(this->memContext, (MemContextCallback)storageWriteRemoteFree, this);
memContextCallbackSet(this->memContext, (MemContextCallback)storageWriteRemoteFree, this);
}
MEM_CONTEXT_TEMP_END();

@@ -6,22 +6,26 @@ Test Memory Contexts
testFree - test callback function
***********************************************************************************************************************************/
MemContext *memContextCallbackArgument = NULL;
bool testFreeThrow = false;

void
testFree(MemContext *this)
testFree(void *thisVoid)
{
MemContext *this = thisVoid;

TEST_RESULT_INT(this->state, memContextStateFreeing, "state should be freeing before memContextFree() in callback");
memContextFree(this);
TEST_RESULT_INT(this->state, memContextStateFreeing, "state should still be freeing after memContextFree() in callback");

TEST_ERROR(
memContextCallback(this, (MemContextCallback)testFree, this),
AssertError, "cannot assign callback to inactive context");
TEST_ERROR(memContextCallbackSet(this, testFree, this), AssertError, "cannot assign callback to inactive context");

TEST_ERROR(memContextSwitch(this), AssertError, "cannot switch to inactive context");
TEST_ERROR(memContextName(this), AssertError, "cannot get name for inactive context");

memContextCallbackArgument = this;

if (testFreeThrow)
THROW(AssertError, "error in callback");
}

/***********************************************************************************************************************************
@@ -236,24 +240,31 @@ testRun(void)
}

// *****************************************************************************************************************************
if (testBegin("memContextCallback()"))
if (testBegin("memContextCallbackSet()"))
{
TEST_ERROR(
memContextCallback(memContextTop(), (MemContextCallback)testFree, NULL), AssertError,
"top context may not have a callback");
memContextCallbackSet(memContextTop(), testFree, NULL), AssertError, "top context may not have a callback");

MemContext *memContext = memContextNew("test-callback");
memContextCallback(memContext, (MemContextCallback)testFree, memContext);
memContextCallbackSet(memContext, testFree, memContext);
TEST_ERROR(
memContextCallback(memContext, (MemContextCallback)testFree, memContext),
AssertError, "callback is already set for context 'test-callback'");
memContextCallbackSet(memContext, testFree, memContext), AssertError,
"callback is already set for context 'test-callback'");

// Clear and reset it
memContextCallbackClear(memContext);
memContextCallback(memContext, (MemContextCallback)testFree, memContext);
memContextCallbackSet(memContext, testFree, memContext);

memContextFree(memContext);
TEST_RESULT_PTR(memContextCallbackArgument, memContext, "callback argument is context");

// Now test with an error
// -------------------------------------------------------------------------------------------------------------------------
TEST_ASSIGN(memContext, memContextNew("test-callback-error"), "new mem context");
testFreeThrow = true;
TEST_RESULT_VOID(memContextCallbackSet(memContext, testFree, memContext), " set callback");
TEST_ERROR(memContextFree(memContext), AssertError, "error in callback");
TEST_RESULT_UINT(memContext->state, memContextStateFree, " check that mem context was completely freed");
}

// *****************************************************************************************************************************
@@ -327,7 +338,7 @@ testRun(void)

TEST_RESULT_BOOL(bCatch, true, "new context error was caught");
TEST_RESULT_PTR(memContextCurrent(), memContextTop(), "context is now 'TOP'");
TEST_RESULT_BOOL(memContext->state == memContextStateFree, true, "new mem context is not active");
TEST_RESULT_UINT(memContext->state == memContextStateFree, true, "new mem context is not active");
}

// *****************************************************************************************************************************

0 comments on commit 7ae9694

Please sign in to comment.
You can’t perform that action at this time.