CODEWITHSUNDEEP
c++multithreadingc++11
lciamp
lciamp

Reputation: 685

Locking a thread while using std::atomic

So in main I have an std::atomic<int>. It gets sent to a thread that generates random numbers:

void wind_thread(std::atomic<int>* wind)
{
    while(total_planes < max_plane )
    {
        *wind = random(1, 360);
    }
}

2 other threads get this variable and do stuff with it, here is an example of one:

void land_thread(std::atomic<int>* wind, std::queue<Plane> *land, Runway *north_south, Runway *east_west)
{
    while(total_planes <= max_plane)
    {
        if(land->size() == 0){}
        else
        {
            std::lock_guard<std::mutex> guard(my_mutex);

            std::cout << "Plane " << land->front().get_plane_id()
            << " landed on " << wind_string(wind) << " Wind was: "
            << *wind << std::endl;

            land->front().set_details(wind_string(wind));

            if((*wind >= 46 && *wind <= 135) || (*wind >= 226 && *wind <= 315))
            {
                east_west->land(land->front());
                land->pop();
            }
            else
            {
                north_south->land(land->front());
                land->pop();
            }

            total_planes++;

        }
    }
}

the wind in the land_thread is fine but when I pass it to that wind_string() function which takes an std::atomic<int>* x, it gets a new value.

It works fine if I put a lock on the wind thread, but I thought the whole point of using std::atomic was that if only one thread is writing to it, I wouldn't need to use a lock. Is it because I'm passing it to a function outside of the thread? I've been looking around online and all I can find is that I shouldn't need the lock. If someone could explain why it behaves like this or has a link that goes a bit more into how atomic behaves, it would be appreciated. Thanks.

Upvotes: 1

Views: 470

Answers (2)

I3ck
I3ck

Reputation: 433

1

your wind thread will use an entire thread at 100% usage just to keep your wind random. Add some sleeps there to avoid unnecessary stress on your CPU or slow downs of your program.
Or consider just generating the random value on demand. (see Why does an empty loop use so much processor time? )

2

atomic only makes the access to the data itself atomic

            if((*wind >= 46 && *wind <= 135) || (*wind >= 226 && *wind <= 315))

still can have wind changed ON EACH check. You could simply copy the current wind beforehand. Something like:

int currentWind = wind->load();
if((currentWind >= 46 && currentWind <= 135) || (currentWind >= 226 && currentWind <= 315))

Or use a mutex as you did before

Upvotes: 0

SergeyA
SergeyA

Reputation: 62573

Since there is no lock around atomic setting of the wind, you have absolutely no guarantee that you are going to see a value wind had at a certain time. All the atomic gives you is that as of the time of setting, this value would be propagated, so that anybody reading it before it is reset will get the set value. It also guarantees anything which is set before this atomic int is set is also going to be visible.

However, in your example, nothing prevents wind from being modified while you hold the lock in another thread.

Upvotes: 2

Related Questions