c++winapicritical-section

Getting Releasing unheld lock CriticalSection in Windows C++ code


I'm getting a warning "Releasing unheld lock 'csCriticalSection'" at the 2nd try below.

Here is my code, but without all the ADO code. I don't know why I'm getting this warning, because if the Execute() function fails, the catch will run a LeaveCriticalSection function. If the Execute() function succeeds, I call LeaveCriticalSection.

#include <windows.h>
#include <synchapi.h>
#include <comdef.h>
#include <conio.h>

CRITICAL_SECTION csCriticalSection;

_ConnectionPtr connectioncreate = nullptr;
_RecordsetPtr recordset = nullptr;

int main()
{
InitializeCriticalSectionAndSpinCount(&csCriticalSection, 4000);

// *** all ADO code here ***
//
// *** all ADO code here ***

BOOL bRanLeaveCriticalSection = FALSE;
try
    {
    EnterCriticalSection(&csCriticalSection);
    // Run an SQL command with ADO.
    /*
    connectioncreate->BeginTrans();
    connectioncreate->Execute(sSQL.data(), nullptr, adCmdText);
    connectioncreate->CommitTrans();
    */
    LeaveCriticalSection(&csCriticalSection);
    bRanLeaveCriticalSection = TRUE;
    }

catch (CONST _com_error& err)
    {
    connectioncreate->CommitTrans();
    if (bRanLeaveCriticalSection == FALSE)
        LeaveCriticalSection(&csCriticalSection);
    }


try
    {
    // From compiler at the next line "if (recordset)":
    // Warning  C26117  Releasing unheld lock 'csCriticalSection'
    if (recordset)
        if (recordset->State == adStateOpen)
            recordset->Close();
    }
catch (CONST _com_error& err)
    {
    err;
    }

_getch();
}

Can anyone help me to for the reason why I'm getting this warning and how to fix it?


Solution

  • You are not initializing csCriticalSection before using it, eg:

    ...
    CRITICAL_SECTION csCriticalSection;
    ...
    
    int main()
    {
        InitializeCriticalSection(&csCriticalSection); // <-- ADD THIS
        ...
        DeleteCriticalSection(&csCriticalSection); // <-- ADD THIS
    }
    

    But also, you really shouldn't be using bRanLeaveCriticalSection at all, this is just bad code design.

    You can use a __try/__finally block instead (if your compiler supports that), eg:

    ...
    CRITICAL_SECTION csCriticalSection;
    ...
    
    int main()
    {
        ...
    
        try
            {
            EnterCriticalSection(&csCriticalSection);
            __try
                {
                // Run an SQL command with ADO.
                }
            __finally
                {
                LeaveCriticalSection(&csCriticalSection);
                }
            }
        catch (CONST _com_error& err)
            {
            ...
            }
    
        ...
    }
    

    However, the best option is to use an RAII wrapper instead, let it call EnterCriticalSection() and LeaveCriticalSection() for you, eg:

    struct CRITICAL_SECTION_LOCK
    {
        CRITICAL_SECTION &m_cs;
    
        CRITICAL_SECTION_LOCK(CRITICAL_SECTION &cs) : m_cs(cs) {
            EnterCriticalSection(&m_cs);
        }
    
        ~CRITICAL_SECTION_LOCK() {
            LeaveCriticalSection(&m_cs);
        }
    };
    
    ...
    
    CRITICAL_SECTION csCriticalSection;
    
    int main()
    {
        ...
    
        try
            {
            CRITICAL_SECTION_LOCK lock(csCriticalSection);
            // Run an SQL command with ADO.
            }
        catch (CONST _com_error& err)
            {
            ...
            }
    
        ...
    }
    

    Or better, use a C++ std::mutex instead, with a proper RAII lock such as std::lock_guard, eg:

    #include <mutex>
    
    std::mutex mtx;
    
    ...
    
    int main()
    {
        try
            {
            std::lock_guard<std::mutex> lock(mtx);
            // Run an SQL command with ADO.
            }
        catch (CONST _com_error& err)
            {
            ...
            }
    
        ...
    }