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

Question

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

rustyx · Accepted Answer · 2020-06-17 11:40:57Z

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

i`ve tried to make some synchronization, as in my edit, but it still generates the same error
@Алексей You forgot to declare the mutex as static too. Only one mutex for only one multimap ;)
@Алексей Not only mutex should be static like the object you want to protect, but you also need to change the scope of protection. Now, you only protect it from simultaneous reading, which means that map can still change while you are reading. I'm not sure if there can be any parallelization at all (unless you are okay with e.g. printing certain states twice while missing others)
I updated the answer to match the edited question. For the next time - it's better to ask a new question instead, to get more focused answers.

Collectives™ on Stack Overflow

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

1 Answer 1

4 Comments

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

4 Comments

Related