what problems can calling std::list/vector/map/deque::empty() outside mutex scope cause?

Question

Consider the following code snippet:

thread 1:

while (true) {
    task = fetch_task();
    {
        lock_guard<mutex> lock(my_mutex);

        // modify content of my_list
        my_list.push_back(task);
    }
}

thread 2:

while (true) {
    if (!my_list.empty()) {
        {
            lock_guard<mutex> lock(my_mutex);

            // modify content of my_list
            if (!my_list.empty()) {
                task = my_list.pop_front();
            }
        }
        if (taks) {
            handle_taks(task);
        }
    }
    do_some_other_stuff();
}

I know this code is poor on many levels (like exception handling) and there are much better approaches when it comes to task handling anyway - I'm just interested in one aspect:

I call my_list.empty() outside the mutex scope (maybe to avoid locking my_mutex in performance critical situations).

That smells bad and I'm not planning to do this - but I wonder what can really happen.empty() returns bool - can I assume at least this call to be safe?

Of course content can have changed after I call empty() so I have to avoid race conditions which I do by again checking empty() inside the mutex scope.

So my question is - what effects can this kind of code have in real world hypothetically regarding false positives, false negatives or even crashes?

You have a data race, so you get undefined behavior. I'm not sure it makes sense making a laundry list of stuff that can happen, since basically anything can happen. It'll be a long list, and the stuff that manifest itself in production with things like that is often not what people think or assume can happen. — nos, Dec 08 '15 at 09:57

score 5 · Answer 1 · answered Dec 08 '15 at 09:58

5

Unsynchronized access to standard library class members from different threads where at least one thread is writing results in a data race (except where explicitly defined not to which is generally not the case, though). A data race is undefined behavior. Undefined behavior can result in arbitrary results.

Put differently: out of your list all can happen. Lots of other things can also happen.

answered Dec 08 '15 at 09:58

Dietmar Kühl

150,225
13
225
380

+1 for this the right way to look at it. For though real problems might be very unlikely - or maybe even impossible - to occur you should never accept undefined behavior. Nevertheless my question is aiming at the potential effects you might observe - maybe just to sensitize oneself to this kind of mistake. – frans Dec 10 '15 at 08:08

score 1 · Answer 2 · answered Dec 08 '15 at 14:36

I believe, knowing possible outcome is useful to trobleshoot - when you witness a particular behaviour, you can guess the reasons. For this reason, I will try to analyze the scenario.

Let's look into chip things first. First of all, standard requires complexity of empty() to be constant - which implies it reads from a single variable (does not iterate the list). It is likely that it is a boolean member inside the list. (After C++ 11 it is most likely to be implemented in terms of size(), but pre C++11 size() is not required to be constant complexity).

Now, if it is a boolean member, it has to be updated on every list modification. As there are no mutexes around variable reading, those updates have the chance of not being visible in reading thread. Torn reads for bools (which are very unlikely to happen anyways, since I know of no architecture which could do this) would not affect this, since it is either yes or no. As a result, you might see the list as empty when it is not.

Now, let's look at C++ level of things. This is also interesting. If there is a slight chance compiler can prove list is empty on the first iteration, the whole if statement will be optimized away.

Conclusion: Likely outcome - nothing happens in the reader, as if list is alway empty.

Despite Dietmar Kühls answer is *correct* it's not answering my question - in your answer you try to elaborate on possible effects/symptomes when you call `empty()` unprotected - which is more what I'm interested in. — frans, Dec 10 '15 at 08:00
Maybe off topic: do you really think the compiler might optimize away the if statement? Have you ever experienced such behavior? Can you provide an example? — frans, Dec 10 '15 at 08:12
@frans, depends on the code. Any example of any use would be too long for stack overflow format. In short - yes, it can when code alllows it. — SergeyA, Dec 10 '15 at 15:24

what problems can calling std::list/vector/map/deque::empty() outside mutex scope cause?

2 Answers2