0

I have this code:

std::map<std::pair<int, int>, std::string> data;
...

ContextMutex context_mutex( _mutex );
auto pair_iterator = data.find( std::make_pair(some, thing));

if(pair_iterator == data.end())
{
    context_mutex.unlock();
    // do some logging without using pair_iterator
}
context_mutex.unlock();

On this code, I only release the mutex for my STL container after checking if pair_iterator == data.end(). I wonder if I can release the mutex before checking pair_iterator == data.end() because I do not use the pair_iterator or any of its contents as I just print a log if the object if found.

On this case, can I release the mutex before checking the condition pair_iterator == data.end() (as the following example) or the access to the data.end() needs to be protected by a mutex too (as done in the first example)?

std::map<std::pair<int, int>, std::string> data;
...

ContextMutex context_mutex( _mutex );
auto pair_iterator = data.find( std::make_pair(some, thing));
context_mutex.unlock();

if(pair_iterator == data.end())
{
    // do some logging without using pair_iterator
}

Related:

  1. Iterator invalidation rules for C++ containers
  2. Why do C++ STL container begin and end functions return iterators by value rather than by constant reference?
  3. STL iterator revalidation for end (past-the-end) iterator?
Evandro Coan
  • 8,560
  • 11
  • 83
  • 144
  • Advise : use std::mutex and std::unique_lock. I don't think releasing the mutex early is a good idea. Imagine you want to use the iterator to modify or lookup data and another thread does something to the map that invalidates the iterator then you still have race conditions. What I would do is get the data out of the map within the lock (copy it) then unlock and then use the copied data for logging (which can be quite slow). – Pepijn Kramer Dec 01 '21 at 14:54
  • 4
    If you don't protect it, then an element could be added to the map before the if statement is executed, giving you a false positive. – NathanOliver Dec 01 '21 at 14:54
  • 1
    Or an element removed, even the element that's just been found. In fact, after the mutex is unlocked the remaining iterator is now completely useless. – Sam Varshavchik Dec 01 '21 at 15:06
  • 2
    Re: "I do not use the `pair_iterator`" -- but you **do** use it: `if(pair_iterator == data.end()) ...`. So, no, you can't unlock before doing the test. – Pete Becker Dec 01 '21 at 15:06
  • You can do `bool is_in_map = (data.find( std::make_pair(some, thing)) != data.end()); context_mutex.unlock(); if (!is_in_map)...` – mch Dec 01 '21 at 15:13

1 Answers1

1

My understanding is, that the end iterator will not be invalidated on insertion or deletion. Thus, it would be safe to compare an iterator to the map's end iterator in a multi-threaded environment.

See also: https://stackoverflow.com/a/1432796/1531765

Iterators in (multi)sets and (multi)maps won't be invalidated in insertions and deletions and thus comparing .end() against previous stored values of .end() will always yield true.

Chris
  • 348
  • 2
  • 6