From 3831b18e0305d31ffe068e94131849c8e8ade4e4 Mon Sep 17 00:00:00 2001 From: Alex Peshkoff Date: Wed, 22 Aug 2018 16:28:27 +0300 Subject: [PATCH] Fixed CORE-5899: Memory leak in GBAK code when used as service --- src/burp/burp.cpp | 25 ++++--------- src/burp/burp.h | 65 +++++++++++++++++++++++---------- src/burp/misc.cpp | 83 ------------------------------------------- src/burp/misc_proto.h | 2 -- src/burp/mvol.cpp | 8 ++--- 5 files changed, 56 insertions(+), 127 deletions(-) diff --git a/src/burp/burp.cpp b/src/burp/burp.cpp index 9db03ec61f..0b93bc2edd 100644 --- a/src/burp/burp.cpp +++ b/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 >( - *getDefaultMemoryPool(), textType, (const UCHAR*) filter.c_str(), + skipDataMatcher.reset(FB_NEW_POOL(tdgbl->getPool()) + Firebird::SimilarToMatcher > + (tdgbl->getPool(), textType, (const UCHAR*) filter.c_str(), filter.length(), '\\', true)); } } diff --git a/src/burp/burp.h b/src/burp/burp.h index b9a3cee2bd..e00312c568 100644 --- a/src/burp/burp.h +++ b/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(&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 diff --git a/src/burp/misc.cpp b/src/burp/misc.cpp index b4fa133edc..66e0053ea0 100644 --- a/src/burp/misc.cpp +++ b/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 diff --git a/src/burp/misc_proto.h b/src/burp/misc_proto.h index 971309e8e1..1bba3073fe 100644 --- a/src/burp/misc_proto.h +++ b/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); diff --git a/src/burp/mvol.cpp b/src/burp/mvol.cpp index 38360afc8e..cb39d286f7 100644 --- a/src/burp/mvol.cpp +++ b/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_crypt_buffer = FB_NEW(UCHAR[ZC_BUFSIZE]); - tdgbl->gbl_decompress = FB_NEW UCHAR[ZC_BUFSIZE]; + tdgbl->gbl_compress_buffer = FB_NEW_POOL(tdgbl->getPool()) UCHAR[ZC_BUFSIZE]; + tdgbl->gbl_crypt_buffer = FB_NEW_POOL(tdgbl->getPool()) UCHAR[ZC_BUFSIZE]; + tdgbl->gbl_decompress = FB_NEW_POOL(tdgbl->getPool()) UCHAR[ZC_BUFSIZE]; }