CODEWITHSUNDEEP
c++c++11boost-mutexstdmutex
Baiqing
Baiqing

Reputation: 1343

Iterating over vector in one thread while other may potentially change reference

I have a vector allow_list that is periodically updated in a thread while another serves a function that checks if a certain string is in that allow_list via:

if (std::find(allow_list->begin(), allow_list->end(), target_string) != allow_list->end()){
    allow = true;
}

Now, the other thread may do something like this

// Some operation to a vector called allow_list_updated
allow_list = allow_list_updated;

Should I add a mutex here to lock and unlock before and after these operations? My intuition tells me it's "ok" and shouldn't crash and burn but this seems like undefined behavior to me.

Upvotes: 0

Views: 185

Answers (2)

user10
user10

Reputation: 296

You have race condition and you need to lock. Simple rule if thread can read variable with non atomic write from another you have race on that variable. Another problem you need to lock all vector. If you have lots of reads and rare writes std::shared_mutex might be good idea. If allow_list that is periodically updated only from the edges, list would be better option for allow_list = allow_list_updated since to swap list you need to swap head and tail. Another potential advantage of list is lack of false sharing. Whatever you do your container and its protection should be in one class.

Upvotes: 1

deviantgeek
deviantgeek

Reputation: 156

when you update a vector, all iterators become invalid. the reason for this is because the vector may reallocate the contents in memory. it may work sometimes, but eventually will segfault when you access an item that moved. the other reason is if you delete elements then your iterator could be pointing out of bounds or skip over entries. so you definitely need to perform some locking in both threads. which kind of lock depends on the rest of your code

i would also recommend either std::swap or std::move instead of allow_list = allow_list_updated; depending on if allow_list_updated can be discarded after the change; it's much faster. if you're updating this list frequently you'll probably want to use std::swap and keep the two lists in scope somewhere and just .clear() and std::swap() them each update. this will combat memory fragmentation. example:

class updater
{
public:
     std::vector<std::string> allowed;
     std::vector<std::string> allowed_updated;

     void update()
     {
          // @TODO: do your update to this->allowed_updated, use this->allowed_updated.reserve() if you know how many items there will be

          std::swap(this->allowed, this->allowed_updated);
          this->allowed_updated.clear();
     }
};

Upvotes: -1

Related Questions