Fixed CORE-5899: Memory leak in GBAK code when used as service

src/burp/burp.cpp

@@ -550,7 +550,7 @@ int gbak(Firebird::UtilSvc* uSvc)
 				// Miserable thing must be a filename
 				// (dummy in a length for the backup file
 
-				file = FB_NEW_POOL(*getDefaultMemoryPool()) burp_fil(*getDefaultMemoryPool());
+				file = FB_NEW_POOL(tdgbl->getPool()) burp_fil(tdgbl->getPool());
 				file->fil_name = str.ToPathName();
 				file->fil_length = file_list ? 0 : MAX_LENGTH;
 				file->fil_next = file_list;
@@ -1386,23 +1386,8 @@ int gbak(Firebird::UtilSvc* uSvc)
 		tdgbl->output_file = NULL;
 	}
 
-	// Free all unfreed memory used by GBAK itself
-	while (tdgbl->head_of_mem_list != NULL)
-	{
-		UCHAR* mem = tdgbl->head_of_mem_list;
-		tdgbl->head_of_mem_list = *((UCHAR **) tdgbl->head_of_mem_list);
-		gds__free(mem);
-	}
-
 	BurpGlobals::restoreSpecific();
 
-#if defined(DEBUG_GDS_ALLOC)
-	if (!uSvc->isService())
-	{
-		gds_alloc_report(0 ALLOC_ARGS);
-	}
-#endif
-
 	return exit_code;
 }
 
@@ -2551,13 +2536,15 @@ void BurpGlobals::setupSkipData(const Firebird::string& regexp)
 			if (!uSvc->utf8FileNames())
 				ISC_systemToUtf8(filter);
 
+			BurpGlobals* tdgbl = BurpGlobals::getSpecific();
 			if (!unicodeCollation)
-				unicodeCollation = FB_NEW UnicodeCollationHolder(*getDefaultMemoryPool());
+				unicodeCollation = FB_NEW_POOL(tdgbl->getPool()) UnicodeCollationHolder(tdgbl->getPool());
 
 			Jrd::TextType* const textType = unicodeCollation->getTextType();
 
-			skipDataMatcher.reset(FB_NEW Firebird::SimilarToMatcher<UCHAR, Jrd::UpcaseConverter<> >(
+			skipDataMatcher.reset(FB_NEW_POOL(tdgbl->getPool())
-				*getDefaultMemoryPool(), textType, (const UCHAR*) filter.c_str(),
+				Firebird::SimilarToMatcher<UCHAR, Jrd::UpcaseConverter<> >
+				(tdgbl->getPool(), textType, (const UCHAR*) filter.c_str(),
 				filter.length(), '\\', true));
 		}
 	}

src/burp/burp.h

@@ -64,21 +64,6 @@
 //#define COMPRESS_DEBUG 1
 #endif // WIRE_COMPRESS_SUPPORT
 
-static inline UCHAR* BURP_alloc(ULONG size)
-{
-	return MISC_alloc_burp(size);
-}
-
-static inline UCHAR* BURP_alloc_zero(ULONG size)
-{
-	return MISC_alloc_burp(size);
-}
-
-static inline void BURP_free(void* block)
-{
-	MISC_free_burp(block);
-}
-
 const int GDS_NAME_LEN		= METADATA_IDENTIFIER_CHAR_LEN * 4 /* max bytes per char */ + 1;
 typedef TEXT			GDS_NAME[GDS_NAME_LEN];
 
@@ -933,29 +918,54 @@ public:
 
 struct BurpCrypt;
 
-class BurpGlobals : public Firebird::ThreadData
+
+class GblPool
+{
+private:
+	// Moved it to separate class in order to ensure 'first create/last destroy' order
+	Firebird::MemoryPool* gbl_pool;
+public:
+	Firebird::MemoryPool& getPool()
+	{
+		fb_assert(gbl_pool);
+		return *gbl_pool;
+	}
+
+	explicit GblPool(bool ownPool)
+		: gbl_pool(ownPool ? MemoryPool::createPool(getDefaultMemoryPool()) : getDefaultMemoryPool())
+	{ }
+
+	~GblPool()
+	{
+		if (gbl_pool != getDefaultMemoryPool())
+			Firebird::MemoryPool::deletePool(gbl_pool);
+	}
+};
+
+class BurpGlobals : public Firebird::ThreadData, public GblPool
 {
 public:
 	explicit BurpGlobals(Firebird::UtilSvc* us)
 		: ThreadData(ThreadData::tddGBL),
-		  defaultCollations(*getDefaultMemoryPool()),
+		  GblPool(us->isService()),
+		  defaultCollations(getPool()),
 		  uSvc(us),
 		  verboseInterval(10000),
 		  flag_on_line(true),
 		  firstMap(true),
 		  stdIoMode(false)
 	{
-		// this is VERY dirty hack to keep current behaviour
+		// this is VERY dirty hack to keep current (pre-FB2) behaviour
 		memset (&gbl_database_file_name, 0,
 			&veryEnd - reinterpret_cast<char*>(&gbl_database_file_name));
 
+		// normal code follows
 		gbl_stat_flags = 0;
 		gbl_stat_header = false;
 		gbl_stat_done = false;
 		memset(gbl_stats, 0, sizeof(gbl_stats));
 		gbl_stats[TIME_TOTAL] = gbl_stats[TIME_DELTA] = fb_utils::query_performance_counter();
 
-		// normal code follows
 		exit_code = FINI_ERROR;	// prevent FINI_OK in case of unknown error thrown
 								// would be set to FINI_OK (==0) in exit_local
 	}
@@ -1274,4 +1284,21 @@ private:
 	const char* format;
 };
 
+static inline UCHAR* BURP_alloc(ULONG size)
+{
+	BurpGlobals* tdgbl = BurpGlobals::getSpecific();
+	return (UCHAR*)(tdgbl->getPool().allocate(size ALLOC_ARGS));
+}
+
+static inline UCHAR* BURP_alloc_zero(ULONG size)
+{
+	BurpGlobals* tdgbl = BurpGlobals::getSpecific();
+	return (UCHAR*)(tdgbl->getPool().calloc(size ALLOC_ARGS));
+}
+
+static inline void BURP_free(void* block)
+{
+	MemoryPool::globalFree(block);
+}
+
 #endif // BURP_BURP_H

src/burp/misc.cpp

@@ -32,89 +32,6 @@
 #include "../burp/misc_proto.h"
 
 
-UCHAR *MISC_alloc_burp(ULONG size)
-{
-/**************************************
- *
- *	M I S C _ a l l o c _ b u r p
- *
- **************************************
- *
- * Functional description
- *	Allocate block of memory. Note that it always zeros out memory.
- *  This could be optimized.
- *
- **************************************/
-
-	BurpGlobals* tdgbl = BurpGlobals::getSpecific();
-
-	// Add some header space to store a list of blocks allocated for this gbak
-	size += ROUNDUP(sizeof(UCHAR *), FB_ALIGNMENT);
-
-	UCHAR* block = (UCHAR*)gds__alloc(size);
-
-	if (!block)
-	{
-		// NOMEM: message & abort FREE: all items freed at gbak exit
-		BURP_error(238, true);
-		// msg 238: System memory exhaused
-		return NULL;
-	}
-
-	memset(block, 0, size);
-
-	// FREE: We keep a linked list of all gbak memory allocations, which
-	// are then freed when gbak exits.  This is important for
-	// NETWARE in particular.
-
-	*((UCHAR**) block) = tdgbl->head_of_mem_list;
-	tdgbl->head_of_mem_list = block;
-
-	return (block + ROUNDUP(sizeof(UCHAR *), FB_ALIGNMENT));
-}
-
-
-void MISC_free_burp( void *free)
-{
-/**************************************
- *
- *	M I S C _ f r e e _ b u r p
- *
- **************************************
- *
- * Functional description
- *	Release an unwanted block.
- *
- **************************************/
-	BurpGlobals* tdgbl = BurpGlobals::getSpecific();
-
-	if (free != NULL)
-	{
-		// Point at the head of the allocated block
-		UCHAR** block = (UCHAR**) ((UCHAR*) free - ROUNDUP(sizeof(UCHAR*), FB_ALIGNMENT));
-
-		// Scan for this block in the list of blocks
-		for (UCHAR **ptr = &tdgbl->head_of_mem_list; *ptr; ptr = (UCHAR **) *ptr)
-		{
-			if (*ptr == (UCHAR *) block)
-			{
-				// Found it - remove it from the list
-				*ptr = *block;
-
-				// and free it
-				gds__free(block);
-				return;
-			}
-		}
-
-		// We should always find the block in the list
-		BURP_error(238, true);
-		// msg 238: System memory exhausted
-		// (too lazy to add a better message)
-	}
-}
-
-
 // Since this code appears everywhere, it makes more sense to isolate it
 // in a function visible to all gbak components.
 // Given a request, if it's non-zero (compiled), deallocate it but

src/burp/misc_proto.h

@@ -24,8 +24,6 @@
 #ifndef BURP_MISC_PROTO_H
 #define BURP_MISC_PROTO_H
 
-UCHAR*	MISC_alloc_burp(ULONG);
-void	MISC_free_burp(void*);
 void	MISC_release_request_silent(Firebird::IRequest*& req_handle);
 int		MISC_symbol_length(const TEXT*, ULONG);
 void	MISC_terminate(const TEXT*, TEXT*, ULONG, ULONG);

src/burp/mvol.cpp

@@ -214,7 +214,7 @@ static Firebird::IKeyHolderPlugin* mvol_get_holder(BurpGlobals* tdgbl, Firebird:
 		if (!keyControl.hasData())
 			(Firebird::Arg::Gds(isc_no_keyholder_plugin) << tdgbl->gbl_sw_keyholder).raise();
 
-		BurpCrypt* g = tdgbl->gbl_crypt = FB_NEW BurpCrypt;
+		BurpCrypt* g = tdgbl->gbl_crypt = FB_NEW_POOL(tdgbl->getPool()) BurpCrypt;
 		g->holder_plugin = keyControl.plugin();
 		g->holder_plugin->addRef();
 
@@ -666,9 +666,9 @@ void MVOL_init(ULONG io_buf_size)
 
 	tdgbl->mvol_io_buffer_size = io_buf_size;
 
-	tdgbl->gbl_compress_buffer = FB_NEW(UCHAR[ZC_BUFSIZE]);
+	tdgbl->gbl_compress_buffer = FB_NEW_POOL(tdgbl->getPool()) UCHAR[ZC_BUFSIZE];
-	tdgbl->gbl_crypt_buffer = FB_NEW(UCHAR[ZC_BUFSIZE]);
+	tdgbl->gbl_crypt_buffer = FB_NEW_POOL(tdgbl->getPool()) UCHAR[ZC_BUFSIZE];
-	tdgbl->gbl_decompress = FB_NEW UCHAR[ZC_BUFSIZE];
+	tdgbl->gbl_decompress = FB_NEW_POOL(tdgbl->getPool()) UCHAR[ZC_BUFSIZE];
 }