4

This is probably a dumb question, but consider the following psuedo-code:

struct Person {
    std::string name;
};

class Registry {
public:
  const std::string& name(int id) const {return _people[id].name;}
  void name(int id, const std::string& name) { [[scoped mutex]]; _people[id].name = name;}

private:
  std::map<int, Person> _people;
};

In this simple example, assume Registry is a singleton that will be accessed by multiple threads. I'm locking during an operation that mutates the data, but not during non-mutating access.

Is this thread safe, or should I also lock during the read operation? I'm preventing multiple threads from trying to modify the data at the same time, but I don't know what would happen if a thread was trying to read at the same time another was writing.

amnesia
  • 1,956
  • 2
  • 18
  • 36
  • "what would happen if a thread was trying to read at the same time another was writing" -- depends on the atomicity of the access. std::map::operator[] is not an atomic operation. – Brian Cain Mar 21 '13 at 15:19
  • 1
    In general, you can't read while you write: you could get corrupt data. But as you said, you can make multiple simultaneous reads. Have a look at [the reader-writer lock](http://en.wikipedia.org/wiki/Readers%E2%80%93writer_lock). – Antoine Mar 21 '13 at 15:19
  • Have a look at [this](http://stackoverflow.com/questions/12033188/how-would-a-readers-writer-lock-be-implemented-in-c11) related question, and at [boost::shared_lock](http://www.boost.org/doc/libs/1_53_0/doc/html/thread/synchronization.html#thread.synchronization.locks.shared_lock) – Antoine Mar 21 '13 at 15:22
  • Brian Cain, I see! Antoine, thank you for showing me that, I can think of lots of cases when this type of lock would be useful. – amnesia Mar 21 '13 at 15:32
  • possible duplicate of [What operations are thread-safe on std::map?](http://stackoverflow.com/questions/2170541/what-operations-are-thread-safe-on-stdmap) – Peter Wood Mar 21 '13 at 22:57
  • Not a dupe, I'm not asking specifically about maps, I just used that for example. – amnesia Mar 22 '13 at 20:28
  • `std::map::operator[]` is a particularly egregious example, though, since what appears to be a read operation can actually have a mutation side-effect: if the key doesn't exist, a new element is created for it. This is horrible API design! – bobince Oct 09 '13 at 10:25

6 Answers6

7

If any thread can modify the data, then you need to lock for all access.

Otherwise, one of your "reading" threads could access the data when it is in an indeterminate state. Modifying a map, for example, requires manipulating several pointers. Your reading thread could acces the map while some - but not all - of the map has been adjusted.

If you can guarantee that the data is not being modified, multiple reads from multiple threads do not need to be locked, however that introduces a fragile scenario that you would have to keep a close eye on.

Drew Dormann
  • 59,987
  • 13
  • 123
  • 180
  • Since I'm trying to learn here, would you mind explaining why? What exactly happens if a thread tries to access the data while another is modifying it? – amnesia Mar 21 '13 at 15:19
  • 2
    @amnesia: According to the C++11 Standard, anything could happen. This is Undefined Behavior. – Andy Prowl Mar 21 '13 at 15:22
4

It's not thread safe to be reading the data while it is being modified. It is perfectly safe to have multiple threads reading the data at once.

This difference is what reader-writer locks are for; they will allow any number of readers but when a writer tries to lock the resource new readers will no longer be allowed and the writer will block until all the current readers are done. Then the writer will proceed and once it's done all the readers will be allowed access again.

The reason it's not safe to read data during modification is that the data can be or can appear to be in an inconsistent state (e.g., the object may temporarily not fulfill invariant). If the reader reads it at that point then it's just like there's a bug in the program failing to keep the data consistent.

// example
int array[10];
int size = 0;

int &top() {
    return array[size-1];
}

void insert(int value) {
    size++;
    top() = value;
}

Any number of threads can call top() at the same time, but if one thread is running insert() then a problem occurs when the lines get interleaved like this:

// thread calling insert         thread calling top
    size++;
                                   return array[size-1];
    array[size-1] = value

The reading thread gets garbage.

Of course this is just one possible way things can go wrong. In general you can't even assume the program will behave as though lines of code on different threads will just interleave. In order to make that assumption valid the language simply tells you that you can't have data races (i.e., what we've been talking about; multiple threads accessing a (non-atomic) object with at least one thread modifying the object)*.

* And for completeness; that all atomic accesses use a sequentially consistent memory ordering. This doesn't matter for you since you're not doing low level work directly with atomic objects.

bames53
  • 86,085
  • 15
  • 179
  • 244
3

Is this thread safe, or should I also lock during the read operation?

It is not thread-safe.

Per Paragraph 1.10/4 of the C++11 Standard:

Two expression evaluations conflict if one of them modifies a memory location (1.7) and the other one accesses or modifies the same memory location.

Moreover, per Paragraph 1.10/21:

The execution of a program contains a data race if it contains two conflicting actions in different threads, at least one of which is not atomic, and neither happens before the other. Any such data race results in undefined behavior. [...]

Andy Prowl
  • 124,023
  • 23
  • 387
  • 451
0

It's not thread safe.

The reading operation could be going through the map (it's a tree) to find the requested object, while a writing operation suddenly adds or removes the something from the map (or worse, the actual point the iterator is at).

If you're lucky you'll get an exception, otherwise it will be just undefined behavior while your map is in an inconsistant state.

Yochai Timmer
  • 48,127
  • 24
  • 147
  • 185
0

I don't know what would happen if a thread was trying to read at the same time another was writing.

Nobody knows. Chaos would ensue.

Multiple threads can share a read-only resource, but as soon as anyone wants to write it, it becomes unsafe for everyone to access it in any way until the writing is done.

Why?

Writes are not atomic. They happen over multiple clock cycles. A process attempting to read an object as it's written may find a half-modified version, temporary garbage.

So

Lock your reads if they are expected to be concurrent with your writes.

salezica
  • 74,081
  • 25
  • 105
  • 166
0

Absolutely not safe!

If you are changing Person::Name from "John Smith" to "James Watt", you may well read back a value of "Jame Smith" or "James mith". Or possibly even something completely different because the way that "change this value for that" may not just copy the new data into the existing place, but entirely replace it with a newly allocated piece of memory that contains some completely undefined content [including something that isn't a valid string].

Mats Petersson
  • 126,704
  • 14
  • 140
  • 227