You've already forked pgbackrest
							
							
				mirror of
				https://github.com/pgbackrest/pgbackrest.git
				synced 2025-10-30 23:37:45 +02:00 
			
		
		
		
	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.
This commit is contained in:
		| @@ -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"); | ||||
|     } | ||||
|  | ||||
|     // ***************************************************************************************************************************** | ||||
|   | ||||
		Reference in New Issue
	
	Block a user