How to get class member variable that require lock without manual lock and unlock?

Question

[EDIT] Does anyone know how to get a class member container that require lock from outside properly? Writing to a class member can be done as below if critical section is used for locking.

EnterCriticalSection(&m_cs);
// write
LeaveCriticalSection(&m_cs);

However, if it is necessary to lookup this container to find data from current data that the container holds. What I could think of was defining Lock, Unlock member function for a class containing the container. And calling them before and after Get.

void Lock(){ EnterCriticalSection(&m_cs); }
std::map<unsigned long long, CObject>& Get() { return m_mapObject; }
void Unlock(){ LeaveCriticalSection(&m_cs); }

This doesn't seem right as far as it is open to error.

What I'm looking for seems to be collaboration of these.

Class variables: public access read-only, but private access read/write

Return locked resource from class with automatic unlocking

Perhaps I'm having difficulty in finding relevant keyword to search.

Below is a small example code for detail.

#include <iostream>
#include <Windows.h>
#include <map>

class CObject
{
private:

    int m_nData;

public:

    CObject()
        : m_nData(0)
    {
    }

    ~CObject()
    {
    }

    void Set(const int _nData) { m_nData = _nData; }
    int Get() { return m_nData; }
};

class CObjectManager
{
private:

    CRITICAL_SECTION m_cs;
    std::map<unsigned long long, CObject> m_mapObject;

public:

    CObjectManager()
        : m_cs{}
        , m_mapObject{}
    {
        InitializeCriticalSection(&m_cs);
    }

    ~CObjectManager()
    {
        DeleteCriticalSection(&m_cs);
    }

    bool Add(const unsigned long long _key, const CObject _object)
    {
        bool bReturn = false;

        EnterCriticalSection(&m_cs);

        if (m_mapObject.find(_key) == m_mapObject.end())
        {
            m_mapObject.insert(std::pair<unsigned long long, CObject>(_key, _object));
            bReturn = true;
        }

        LeaveCriticalSection(&m_cs);

        return bReturn;
    }

    bool Delete(const unsigned long long _key)
    {
        bool bReturn = false;

        EnterCriticalSection(&m_cs);

        std::map<unsigned long long, CObject>::iterator iter = m_mapObject.find(_key);
        if (iter != m_mapObject.end())
        {
            m_mapObject.erase(iter);
            bReturn = true;
        }

        LeaveCriticalSection(&m_cs);

        return bReturn;
    }

    void Lock(){ EnterCriticalSection(&m_cs); }
    std::map<unsigned long long, CObject>& Get() { return m_mapObject; }
    void Unlock(){ LeaveCriticalSection(&m_cs); }
};

void DoSomething(CObject* _pObject)
{
    int a = 10;
}

CObjectManager objectManager{};

DWORD Thread(LPVOID _pParam)
{
    for (unsigned long long i = 0ull; i < 100000ull; ++i)
    {
        CObject object{};
        object.Set(i);
        objectManager.Add(i, object);
    }

    return 0;
}

int main()
{
    CObject object0;
    object0.Set(1);

    CObject object1;
    object1.Set(2);

    bool b = objectManager.Add(3ull, object0);
    bool b1 = objectManager.Add(3ull, object1);
    bool b2 = objectManager.Add(4ull, object1);
    bool b3 = objectManager.Delete(3ull);

    CreateThread(nullptr, 0, &Thread, nullptr, 0, nullptr);
    Sleep(0);

    // Need to get current COject map of CObjectManager
    CObject object2{};
    std::map<unsigned long long, CObject>::iterator iter;

    while (true)
    {
        objectManager.Lock();
        std::map<unsigned long long, CObject> mapObject = objectManager.Get();
        iter = mapObject.find(90000ull);
        if (iter != mapObject.end())
        {
            object2.Set(iter->second.Get());
            objectManager.Unlock();
            break;
        }
        objectManager.Unlock();
    }

    DoSomething(&object2);

    Sleep(2000);

    return 0;
}

Any comment or answer will be appreciated.

the question is unclear. You should take a look at RAII style locking of mutexes as eg with [`std::unique_lock`](https://en.cppreference.com/w/cpp/thread/unique_lock) — 463035818_is_not_an_ai, Nov 29 '22 at 12:24
"This doesn't seem right as far as it is open to error." what exactly do you mean? If you refer to a missiing unlock due to an exception being thrown or because it simply has been forgotten, then RAII-style locking is the answer — 463035818_is_not_an_ai, Nov 29 '22 at 12:26
@463035818_is_not_a_number Thanks for your comment. Yes what I meant by open to error was missing unlock or locking twice kind of errors. — YoonSeok OH, Nov 29 '22 at 12:34
then you should [edit](https://stackoverflow.com/posts/74614005/edit) the quesiton to clarify that. Currently its not clear what the question is. The only question is in the title, and actually I dont quite understand how it relates to the rest of the quesiton — 463035818_is_not_an_ai, Nov 29 '22 at 12:39
@YoonSeokOH Usually we solve this by, instead of having `Get()` on the interface, having `Lock()` return an object that has a lock and the result of `Get()`; then the dtor of this object is what's now in `Unlock()`. If that's ok for you, I can turn it to an answer. — lorro, Nov 29 '22 at 12:47
@463035818_is_not_a_number I've edited question. Does it look clearer? — YoonSeok OH, Nov 29 '22 at 12:49
@lorro Thanks for your comment! Wait, I'm having trouble understanding "having Lock() return an object that has a lock and the result of Get(); then the dtor of this object is what's now in Unlock()". Could you please elaborate a little bit more? — YoonSeok OH, Nov 29 '22 at 12:52
@lorro Does it mean something like Get() returns a new defined object that has lock and address of map, and the new object automatically call Lock() in constructor and call Unlock() in destructor? — YoonSeok OH, Nov 29 '22 at 13:02
@YoonSeokOH I've tried to build it as an answer, please check if it's what you're looking for. (If it's not that, feel free to tell, then I delete it and we keep looking.) — lorro, Nov 29 '22 at 13:03

score 1 · Accepted Answer · answered Nov 29 '22 at 13:02

If what concerns you is caller potentially missing lock/unlock calls, then a RAII pattern might be of help:

class CObjectManager
{
private:

    CRITICAL_SECTION m_cs;
    std::map<unsigned long long, CObject> m_mapObject;

public:

    CObjectManager()
        : m_cs{}
        , m_mapObject{}
    {
        InitializeCriticalSection(&m_cs);
    }

    ~CObjectManager()
    {
        DeleteCriticalSection(&m_cs);
    }

    auto LockAndGet()
    {
        EnterCriticalSection(&m_cs);
        struct LockAndGetResult
        {
        private:
            CRITICAL_SECTION& m_cs;

        public:
            std::map<unsigned long long, CObject> m_mapObject;

            LockAndGetResult(CRITICAL_SECTION& cs, std::map<unsigned long long, CObject>& mapObject) : m_cs(cs), m_mapObject(mapObject) {}

            ~LockAndGetResult() { LeaveCriticalSection(&m_cs); }
        };

        return LockAndGetResult(m_cs, m_mapObject);
    }
};

// usage:

CObjectManager co;
{
    auto maplock = co.LockAndGet();
    doSomethingWith(maplock.m_mapObject);
} // maplock dtor releases lock

Oh yeah. This looks easier and safer. Perhaps I couldn't think of this approach because I'm not familiar with RAII pattern. std::unique_ptr underlying mechanism also seems same. I really thank your kind and elaborate wording and answer. Yeah, this is what I was looking for. It's great of you understood despite my poor expression. — YoonSeok OH, Nov 29 '22 at 13:15

How to get class member variable that require lock without manual lock and unlock?

1 Answers1