CODEWITHSUNDEEP
c++multithreadingc++11race-condition
mynameisjohnj
mynameisjohnj

Reputation: 431

Multithreaded program works only with print statements

I wish I could have thought of a more descriptive title, but that is the case. I've got some code that I'd like to do some image processing with. I also need to get some statistical data from those processed images, and I'd like to do that on a separate thread so my main thread can keep doing image processing.

All that aside, here's my code. It shouldn't really be relevant, other than the fact that my Image class wraps an OpenCV Mat (though I'm not using OMP or anything, as far as I'm aware):

#include <thread>
#include <iostream>
#include <vector>
using namespace std;

//Data struct
struct CenterFindData{
    //Some images I'd like to store
    Image in, bpass, bpass_thresh, local_max, tmp;
    //Some Data (Particle Data = float[8])
    vector<ParticleData> data;
    //My thread flag
    bool goodToGo{ false };
    //Constructor
    CenterFindData(const Image& m);
};

vector<ParticleData> statistics(CenterFindData& CFD);
void operate(vector<CenterFindData> v_CFD);


..........................................
..........................................
..........................................


void operate(vector<CenterFindData> v_CFD){
    //Thread function, gathers statistics on processed images
    thread T( [&](){
        int nProcessed(0);
        for (auto& cfd : v_CFD){
            //Chill while the images are still being processed
            while (cfd.goodToGo == false){ 
                 //This works if I uncomment this print statement
                /*cout << "Waiting" << endl;*/ 
            }
            cout << "Statistics gathered from " << nProcessed++ << " images" << endl;
            //This returns vector<ParticleData>
            cfd.data = m_Statistics(cfd);
        }
    });

    //Run some filters on the images before statistics
    int nProcessed(0);
    for (auto& cfd : v_CFD){
        //Preprocess images
        RecenterImage(cfd.in);
        m_BandPass(cfd);
        m_LocalMax(cfd);
        RecenterImage(cfd.bpass_thresh);
        //Tell thread to do statistics, on to the next
        cfd.goodToGo = true;
        cout << "Ran filters on " << nProcessed++ << " images" << endl;
    }

    //Join thread
    T.join();
}

I figure that the delay from cout is avoid some race condition I otherwise run into, but what? Because only one thread modified the bool goodToGo, while the other checks it, should that be a thread safe way of "gating" the two functions?

Sorry if anything is unclear, I'm very new to this and seem to make a lot of obvious mistakes WRT multithreaded programming.

Thanks for your help

Upvotes: 1

Views: 1082

Answers (1)

Mats Petersson
Mats Petersson

Reputation: 129344

When you have:

 while (cfd.goodToGo == false){  }

the compiler doesn't see any reason to "reload" the value of goodToGo (it doesn't know that this value is affected by other threads!). So it reads it once, and then loops forever.

The reason printing something makes a difference is, that the compiler doesn't know what the print function actually will and won't affect, so "just in case", it reloads that value (if the compiler could "see inside" all of the printing code, it could in fact decide that goodToGo is NOT changed by the printing, and not needing to reload - but there are limits to how much time [or some proxy for time, such as "number of levels of calls" or "number of intermediate instructions"] the compiler spends on figuring these sort of thing out [and there may of course be calls to code that the compiler doesn't actually have access to the source code of, such as the system calls to write or similar].

The solution, however, is to use thread-safe mechanisms to update the goodToGo - we could just throw a volatile attribute to the variable, but that will not guarantee that, for example, another processor gets "told" that the value has updated, so could delay the detection of the updated value significantly [or even infinitely under some conditions].

Use std::atomic_bool goodToGo and the store and load functions to access the value inside. That way, you will be guaranteed that the value is updated correctly and "immediately" (as in, a few dozen to hundreds of clock-cycles later) seen by the other thread.

As a side-note, which probably should have been the actual answer: Busy-waiting for threads is a bad idea in general, you should probably look at some thread-primitives to wait for a condition_variable or similar.

Upvotes: 5

Related Questions