Bug #43995 Falcon Valgrind warnings in StorageInterface, StorageHandler & CmdGen

Description:
Kelly Long wrote:
---------------------------------------------------
==24131== 178,030 bytes in 1,156 blocks are possibly lost in loss record 5 of 5
==24131==    at 0x4A0739E: malloc (vg_replace_malloc.c:207)
==24131==    by 0x942EA0: MemMgr::allocate(unsigned long) (MemMgrV0SysDefault.cpp:320)
==24131==    by 0x9421AE: Log::addListener(int, void (*)(int, char const*, void*), void*) (MemoryManager.h:109)
==24131==    by 0x8CB0A2: StorageInterface::falcon_init(void*) (ha_falcon.cpp:254)
==24131==    by 0x7A8A55: ha_initialize_handlerton(st_plugin_int*) (handler.cc:448)
==24131==    by 0x8317C1: _ZL17plugin_initializeP13st_plugin_int (sql_plugin.cc:1008)
==24131==    by 0x83471B: plugin_init(int*, char**, int) (sql_plugin.cc:1221)
==24131==    by 0x6A5288: _ZL22init_server_componentsv (mysqld.cc:4079)
==24131==    by 0x6A8247: main (mysqld.cc:4593)

---------------------------------------------------
090327 19:45:55 [Note] Event Scheduler: Purging the queue. 0 events
==10163== Invalid read of size 4
==10163==    at 0x8D353C: StorageDatabase::release() (StorageDatabase.cpp:949)
==10163==    by 0x8D9AC1: StorageTableShare::~StorageTableShare() (StorageTableShare.cpp:161)
==10163==    by 0x8D5530: StorageHandler::~StorageHandler() (StorageHandler.cpp:157)
==10163==    by 0x8D3975: freeFalconStorageHandler (StorageHandler.cpp:118)
==10163==    by 0x8CACBA: StorageInterface::panic(handlerton*, ha_panic_function) (ha_falcon.cpp:297)
==10163==    by 0x7A755B: ha_finalize_handlerton(st_plugin_int*) (handler.cc:393)

==10163==  Address 0x4d73e18 is 400 bytes inside a block of size 408 free'd
==10163==    at 0x4A0609F: free (vg_replace_malloc.c:323)
==10163==    by 0x942E44: MemMgrPoolRelease(MemMgr*, void*) (MemMgrV0SysDefault.cpp:342)
==10163==    by 0x8D54F5: StorageHandler::~StorageHandler() (MemoryManager.h:126)
==10163==    by 0x8D3975: freeFalconStorageHandler (StorageHandler.cpp:118)
==10163==    by 0x8CACBA: StorageInterface::panic(handlerton*, ha_panic_function) (ha_falcon.cpp:297)
==10163==    by 0x7A755B: ha_finalize_handlerton(st_plugin_int*) (handler.cc:393)

---------------------------------------------------
==19425== Mismatched free() / delete / delete []
==19425==    at 0x4A05B9D: operator delete(void*) (vg_replace_malloc.c:342)
==19425==    by 0x90623B: CmdGen::~CmdGen() (CmdGen.cpp:150)
==19425==    by 0x8CE102: StorageInterface::createIndex(char const*, char const*, TABLE*, int) (ha_falcon.cpp:972)
==19425==    by 0x8CE31E: StorageInterface::addIndex(THD*, TABLE*, st_ha_create_information*, st_ha_alter_information*, Bitmap<39>*) (ha_falcon.cpp:2616)
==19425==    by 0x8CE3DB: StorageInterface::alter_table_phase2(THD*, TABLE*, st_ha_create_information*, st_ha_alter_information*, Bitmap<39>*) (ha_falcon.cpp:2537)
==19425==    by 0x7C1CFC: mysql_fast_or_online_alter_table(THD*, TABLE_LIST*, TABLE*, st_ha_create_information*, st_ha_alter_information*, Bitmap<39>*, enum_enable_or_disable) (sql_table.cc:6000)
==19425==    by 0x7C5BBF: mysql_alter_table(THD*, char*, char*, st_ha_create_information*, TABLE_LIST*, Alter_info*, unsigned, st_order*, bool) (sql_table.cc:7166)
==19425==    by 0x6B800C: mysql_execute_command(THD*) (sql_parse.cc:2804)

==19425==  Address 0x104d94e8 is 0 bytes inside a block of size 1,143 alloc'd
==19425==    at 0x4A0661C: operator new[](unsigned long) (vg_replace_malloc.c:274)
==19425==    by 0x905FD5: CmdGen::getString() (CmdGen.cpp:117)
==19425==    by 0x8CE14E: StorageInterface::createIndex(char const*, char const*, TABLE*, int) (ha_falcon.cpp:960)
==19425==    by 0x8CE31E: StorageInterface::addIndex(THD*, TABLE*, st_ha_create_information*, st_ha_alter_information*, Bitmap<39>*) (ha_falcon.cpp:2616)
==19425==    by 0x8CE3DB: StorageInterface::alter_table_phase2(THD*, TABLE*, st_ha_create_information*, st_ha_alter_information*, Bitmap<39>*) (ha_falcon.cpp:2537)
==19425==    by 0x7C1CFC: mysql_fast_or_online_alter_table(THD*, TABLE_LIST*, TABLE*, st_ha_create_information*, st_ha_alter_information*, Bitmap<39>*, enum_enable_or_disable) (sql_table.cc:6000)
==19425==    by 0x7C5BBF: mysql_alter_table(THD*, char*, char*, st_ha_create_information*, TABLE_LIST*, Alter_info*, unsigned, st_order*, bool) (sql_table.cc:7166)
==19425==    by 0x6B800C: mysql_execute_command(THD*) (sql_parse.cc:2804)

How to repeat:
Run valgrind against DBT2

Suggested fix:
Kelly Long sugggested these fixes
---------------------------------------------------
storage/falcon/ha_falcon.cpp: near line 287
#define KELLY_VALGRIND_FIX_LOGGER
int StorageInterface::falcon_deinit(void *p)
{
    if(storageHandler)
        {
#ifdef KELLY_VALGRIND_FIX_LOGGER
        storageHandler->deleteNfsLogger(StorageInterface::mysqlLogger, NULL); // KELLY - fix memory leak (found via valgrind)
        storageHandler->deleteNfsLogger(StorageInterface::logger, NULL); // KELLY - fix memory leak (found via valgrind)
#endif // KELLY_VALGRIND_FIX
        storageHandler->shutdownHandler();
        freeFalconStorageHandler();
        }
    return 0;
}

---------------------------------------------------
storage/falcon/StorageHandler.cpp near line 146:
#define KELLY_VALGRIND_FIX
StorageHandler::~StorageHandler(void)
{
#ifdef KELLY_VALGRIND_FIX
    // KELLY - delete StorageTableShares FIRST as they rely on StroageDatabase (valgrind found this)
    for (int n = 0; n < tableHashSize; ++n)
        for (StorageTableShare *table; (table = tables[n]);)
            {
            tables[n] = table->collision;
            delete table;
            }
    for (int n = 0; n < databaseHashSize; ++n)
        for (StorageDatabase *storageDatabase; (storageDatabase = storageDatabases[n]);)
             {
            storageDatabases[n] = storageDatabase->collision;
            delete storageDatabase;
            }
#endif // KELLY_VALGRIND_FIX

---------------------------------------------------
storage/falcon/CmdGen.cpp near end of file:
#define KELLY_VALGRIND_FIX_CMDGEN
const char* CmdGen::getString(void)
{
    if (!hunks && remaining)
        {
        *ptr = 0;

        return buffer;
        }
#ifdef KELLY_VALGRIND_FIX_CMDGEN
    delete [] temp; // KELLY - memory leak fix (found via valgrind)
#else // KELLY_VALGRIND_FIX_CMDGEN
    delete temp;
#endif // KELLY_VALGRIND_FIX_CMDGEN
    temp = new char[totalLength + 1];
    memcpy(temp, buffer, sizeof(buffer));
    char *p = temp + sizeof(buffer);
    if (hunks)
        {
        for (CmdHunk *hunk = hunks; hunk->next; hunk = hunk->next)
            {
            memcpy (p, hunk->data, sizeof(hunk->data));
            p += sizeof(hunk->data);
            }

        size_t len = sizeof(currentHunk->data) - remaining;
        memcpy(p, currentHunk->data, len);
        p += len;
        }

    *p = 0;

    return temp;
}

void CmdGen::reset(void)
{
    for (CmdHunk *hunk; (hunk = hunks);)
        {
        hunks = hunk->next;
        delete hunk;
        }

#ifdef KELLY_VALGRIND_FIX
    delete [] temp; // KELLY - memory leak fix (found via valgrind)
#else // KELLY_VALGRIND_FIX
    delete temp;
#endif // KELLY_VALGRIND_FIX
    init();
}

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/70947

Changes by Kelly Long were pushed by Kevin Lewis

Pushed into 6.0.11-alpha (revid:hky@sun.com-20090402144811-yc5kp8g0rjnhz7vy) (version source revid:hky@sun.com-20090331202425-9j6894frsl0m0frp) (merge vers: 6.0.11-alpha) (pib:6)

A note has been added to the 6.0.11 changelog: 

valgrind would report errors for the StorageInterface, StorageHAndler and CmdGen portions of Falcon.