CODEWITHSUNDEEP
c++stlmultimap
Olexy
Olexy

Reputation: 323

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

Upvotes: 1

Views: 274

Answers (1)

rustyx
rustyx

Reputation: 85286

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();
}

Upvotes: 4

Related Questions