C++ thread attach/dettach segfaults

Question

I use a plugin written in C++ for running queries on MySQL. It's used inside a Xojo (www.xojo.com) made application.

The problem is that if too many queries are executed too often it crashes on linux with a segmentation fault.

The plugin itself works by detaching from the calling thread before executing the query in order to not block the main application etc and then re-attaching once it's done. I think this re-attaching is the problem (gdb debugging in linux seems like this) but due to not having symbols on the Xojo's framework I'm not so sure.

This are the two methods/functions used for detaching and re-attaching

void ReattachCurrentThread(void *token)
{
    static void (*pAttachThread)(void*) = nullptr;
    if (!pAttachThread)
        pAttachThread = (void (*)(void *)) gResolver("_UnsafeAttachCurrentThread");
    if (pAttachThread) pAttachThread( token );
}

void * DetachCurrentThread(void)
{
    static void * (*pDetachThread)(void) = nullptr;
    if (!pDetachThread)
        pDetachThread = (void * (*)(void)) gResolver("_UnsafeDetachCurrentThread");
    if (pDetachThread) return pDetachThread();
    return nullptr;
}

And here is one place where those are called:

REALdbCursor MySQLPerformSelect(MySQLDatabaseData *db, REALstring queryStr)
{
    if (db->fConnection == nullptr) return nullptr;

    if (!LockDatabaseUsage( db )) return nullptr;

    REALstringData stringData;
    if (!REALGetStringData( queryStr, REALGetStringEncoding( queryStr ), &stringData )) return nullptr;

    void *detachToken = DetachCurrentThread();
    int err = mysql_real_query( db->fConnection, (const char *)stringData.data, stringData.length );
    ReattachCurrentThread( detachToken );
    db->CaptureLastError();

    REALDisposeStringData( &stringData );

    REALdbCursor retCursor = nullptr;
    if (0 == err) {
        // Allocate a cursor
        MySQLCursorData *curs = new MySQLCursorData;
        bzero( curs, sizeof( MySQLCursorData ) );

        curs->fCursor = new MySQLCursor( db );

        retCursor = NewDBCursor( curs );
    }

    UnlockDatabaseUsage( db );

    return retCursor;
}

My question is: is there anything wrong with the code above and is it expected to cause a segfault because it's not being careful somehow etc? I'm not a C++ programmer but it seems too blunt in my understanding, like not trying to see if thread is available first etc. Again, I'm not a C++ programmer so all I'm saying may be absurd etc...

The "whole" plugin's code is here: plugin's source

Answer 1

There are at least 2 problems with the code:

1. The calls to ReattachCurrentThread()/DetachCurrentThread() are not synchronized
2. UnlockDatabaseUsage() is not always called: function MySQLPerformSelect() can return without calling UnlockDatabaseUsage()

The first problem can be fixed as follows:

#include <mutex>

std::mutex g_attachmentMutex;
void ReattachCurrentThread(void *token)
{
    std::lock_guard<std::mutex> mlg(g_attachmentMutex);
    static void (*pAttachThread)(void*) = nullptr;
    if (!pAttachThread)
        pAttachThread = (void (*)(void *)) gResolver("_UnsafeAttachCurrentThread");
    if (pAttachThread) pAttachThread( token );
}

void * DetachCurrentThread(void)
{
    std::lock_guard<std::mutex> mlg(g_attachmentMutex);
    static void * (*pDetachThread)(void) = nullptr;
    if (!pDetachThread)
        pDetachThread = (void * (*)(void)) gResolver("_UnsafeDetachCurrentThread");
    if (pDetachThread) return pDetachThread();
    return nullptr;
}

The second problem can be fixed as follows:

class MySQLPerformSelectCleaner
{
    MySQLDatabaseData *_db;
public:
    MySQLPerformSelectCleaner(MySQLDatabaseData *db)
        : _db(db)
    {
    }
    ~MySQLPerformSelectCleaner()
    {
        UnlockDatabaseUsage(_db);
    }
};
REALdbCursor MySQLPerformSelect(MySQLDatabaseData *db, REALstring queryStr)
{
    if (db == nullptr || db->fConnection == nullptr) return nullptr;

    if (!LockDatabaseUsage( db )) return nullptr;
    MySQLPerformSelectCleaner c(db);

    REALstringData stringData;
    if (!REALGetStringData( queryStr, 
        REALGetStringEncoding( queryStr ), &stringData )) 
    {
        return nullptr;
    }

    void *detachToken = DetachCurrentThread();
    // perhaps a check for detachToken==nullptr is needed here
    int err = mysql_real_query( db->fConnection, (const char *)stringData.data, stringData.length );
    ReattachCurrentThread( detachToken );
    db->CaptureLastError();

    REALDisposeStringData( &stringData );

    REALdbCursor retCursor = nullptr;
    if (0 == err) {
        // Allocate a cursor
        MySQLCursorData *curs = new MySQLCursorData;
        bzero( curs, sizeof( MySQLCursorData ) );

        curs->fCursor = new MySQLCursor( db );

        retCursor = NewDBCursor( curs );
    }

    // Rely on the cleaner to call this: UnlockDatabaseUsage( db );
    return retCursor;
}