Search code examples
c++stlmultimap

c++ small chance for runtime error while using std::multimap


Sometimes i get runtime error while using multimap std::async. Visual2019 in debug mode show this error:

Expression: cannot dereference end map/set iterator.

Example of code that generates error:

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

class MyClass
{
public:
    MyClass()
    {

        mp.emplace(mapsize, 'f');
        mapsize += 1;
        ft = std::async([this]() {
            mx.lock();
            while (true) {
                for (int i = 0; i < mapsize; i++) {
                    auto pr = mp.equal_range(i);
                    for (auto j = pr.first; j != pr.second; j++)
                        std::cout << j->second << "\n";}}
            mx.unlock(); });
    }
private:
    std::mutex mx;
    static int mapsize;
    std::future <void>ft;
    static std::multimap <int, char> mp;
};
int MyClass::mapsize;
std::multimap <int, char> MyClass::mp;


int main()
{
    for (int i = 0; i < 100000; i++)
        new MyClass();
}

Edit: i have made some synchronization, but it still generates the same error


Solution

  • std::async by default runs in a separate thread. So you're accessing the same object (mp) from multiple threads without synchronization. This is known as a race condition, a form of undefined behavior.

    Unsynchronized parallel access to the same object is possible only in case of (1) multiple readers, 0 writers, or (2) 0 readers, 1 writer. In all other cases access should be serialized, for example by using a mutex.

    But note that when using a mutex, all access to the shared object must be protected by the same mutex. So the mutex should be made static and be used around mp.emplace and mapsize += 1, too.

    Also, for better exception safety, use unique_lock or lock_guard (RAII) instead of locking the mutex manually:

    class MyClass
    {
    public:
        MyClass()
        {
            std::lock_guard<std::mutex> lock(mtx);
            mp.emplace(mapsize, 'f');
            mapsize += 1;
            ft = std::async([this]() {
                while (true) {
                    std::lock_guard<std::mutex> lock(mtx);
                    for (int i = 0; i < mapsize; i++) {
                        auto pr = mp.equal_range(i);
                        for (auto j = pr.first; j != pr.second; j++)
                            std::cout << j->second << "\n";
                    }
                }
            });
        }
    private:
        std::future <void>ft;
        static std::mutex mtx; // protects everything from here on down
        static int mapsize;
        static std::multimap <int, char> mp;
    };
    int MyClass::mapsize;
    std::mutex MyClass::mtx;
    std::multimap <int, char> MyClass::mp;
    
    int main()
    {
        for (int i = 0; i < 100000; i++)
            new MyClass();
    }