Visual C++ and concurrency threads..why this happened?

Question

I have to write a program that works with threads in concurrency, for processing OpenCV Mat images. Every thread pick an image from a queue, processing it and put result to another queue. I use a thread safe template queue (as you can see in code) of Mat images.

But the strange behaviour of threads is: If I launch program multiple times, every time I obtain a different result in numbers of elaborations of single thread (the counter "add" that I've inserted to monitoring number of images processing by single threads).

The first thread (zero) always doing all it's elaborations (in this example, 10) but the rest of threads, don't. Sometimes every thread doing 10 elaborations, sometimes 3, some times 5...2..with the new changes (CONDITION VARIABLES and CRITICAL SECTION) threads doing only 1 operation.

I don't know where is the problem...why this happened.

I past here my code, and ask you to check it and tell me in your opinion, what's the problem...I'm desperate.

This is the code:

#include <opencv\cv.h>
#include <opencv\highgui.h>
#include <opencv2\highgui\highgui.hpp>
#include <stdio.h>
#include <stdlib.h>
#include <windows.h>
#include <process.h>
#include <queue>

using namespace std;
using namespace cv;

/*thread safe queue*/

template<typename T>
class coda_concorr
{
private:
    std::queue<T> la_coda;
    HANDLE mutex;

public: 
    bool elemento;
    coda_concorr()
    {
        mutex = CreateMutex(NULL,FALSE,NULL);
        }
    ~coda_concorr()
    {}
    void push(T& data)
    {
        WaitForSingleObject(mutex,INFINITE);
        la_coda.push(data);
        ReleaseMutex(mutex);
        }
    bool vuota() const
    {
        WaitForSingleObject(mutex,INFINITE);
        bool RetCode = la_coda.empty();
        ReleaseMutex(mutex);
        return RetCode;
    }
    bool try_pop(T& popped)
    {
        WaitForSingleObject(mutex,INFINITE);
        while (la_coda.empty()){
            ReleaseMutex(mutex);
            return false;
        }
        WaitForSingleObject(mutex,INFINITE);
        popped = la_coda.front();
        la_coda.pop();
        ReleaseMutex(mutex);
        return true;
    }
};


struct Args
{
    coda_concorr<cv::Mat> in;
    coda_concorr<cv::Mat> *out; //puntatore a coda successiva
};


CONDITION_VARIABLE NonVuoto1;
CONDITION_VARIABLE NonVuoto2;
CONDITION_VARIABLE NonVuoto3;
CONDITION_VARIABLE NonVuoto4;
CRITICAL_SECTION  Lock1;
CRITICAL_SECTION  Lock2;
CRITICAL_SECTION  Lock3;
CRITICAL_SECTION  Lock4;

bool stop;

//initial populating queue
void puts (void* param){
    Args* arg = (Args*)param;
    int i=0;
    Mat image;

    while(!arg->in.vuota()){
        arg->in.try_pop(image);
        arg->out->push(image);
        i++;        
        WakeConditionVariable(&NonVuoto1);
        }
    //fine  
    cout<<endl<<"Thread (PUSH) terminato con "<<i<<" elaborazioni."<<endl;
    WakeConditionVariable(&NonVuoto1);
    _endthread();
}

//grey funct
void grey (void *param){
    Mat temp1,temp2;
    int add = 0;
    Args* arg = (Args*)param;
    while(true){
        EnterCriticalSection(&Lock1);
        //se vuoto
        while(arg->in.vuota() && !stop){
             SleepConditionVariableCS(&NonVuoto1,&Lock1,INFINITE);
            }
            if(stop==true){
            LeaveCriticalSection(&Lock1);
            break;
            }
        arg->in.try_pop(temp1);
        cvtColor(temp1,temp2,CV_BGR2GRAY);
        arg->out->push(temp2);
        add++;
        cout<<endl<<"grey ha fatto: "<<add<<endl;
        LeaveCriticalSection(&Lock1);
        WakeConditionVariable(&NonVuoto2);
        }
    //fine  
    cout<<endl<<"Thread (GREY) terminato con "<<add<<" elaborazioni."<<endl;
    _endthread();
}

//threshold funct
void soglia(void *param){
    Mat temp1a,temp2a;
    int add=0;
    Args* arg = (Args*)param;
    while(true){
        EnterCriticalSection(&Lock2);
        while(arg->in.vuota() && stop == false){
             SleepConditionVariableCS(&NonVuoto2,&Lock2,INFINITE);
            }
        if(stop==true){
            LeaveCriticalSection(&Lock2);
            break;
            }
        arg->in.try_pop(temp1a);
        threshold(temp1a,temp2a,128,255,THRESH_BINARY);
        arg->out->push(temp2a);
        add++;
        LeaveCriticalSection(&Lock2);
        WakeConditionVariable(&NonVuoto3);
        cout<<endl<<"soglia ha fatto: "<<add<<endl;
        }
        //fine 
     cout<<endl<<"Thread (SOGLIA) terminato con "<<add<<" elaborazioni."<<endl;
     _endthread();
}

//erode/dilate funct
void finitura(void *param){
    Mat temp1b,temp2b,temp2c;
    int add = 0;
    Args* arg = (Args*)param;
    //come consumatore
    while(true){
        EnterCriticalSection(&Lock3);
        while(arg->in.vuota() && stop == false){
             SleepConditionVariableCS(&NonVuoto3,&Lock3,INFINITE);
            }
        if(stop==TRUE){
            LeaveCriticalSection(&Lock3);
            break;
            }   
        arg->in.try_pop(temp1b);
        erode(temp1b,temp2b,cv::Mat());
        dilate(temp2b,temp2c,Mat());
        arg->out->push(temp2c);
        add++;
        LeaveCriticalSection(&Lock3);
        WakeConditionVariable(&NonVuoto4);
        cout<<endl<<"erode ha fatto: "<<add<<endl;
        }
     //fine 
     cout<<endl<<"Thread (ERODE) terminato con "<<add<<" elaborazioni."<<endl;
    _endthread();
}

//contour funct
void contorno (void *param){
    Mat temp;
    int add=0;
    Args* arg = (Args*)param;
    //come consumatore
    while(true){
        EnterCriticalSection(&Lock4);
        while(arg->in.vuota() && stop == false){
             SleepConditionVariableCS(&NonVuoto4,&Lock4,INFINITE);
            }
        if(stop==TRUE){
            LeaveCriticalSection(&Lock4);
            break;
            }   
    //esegue pop
    arg->in.try_pop(temp);
    //trova i contorni
    vector<vector<Point>> contorni;
    findContours(temp,contorni,CV_RETR_LIST, CV_CHAIN_APPROX_SIMPLE);
    //disegna i contoni in un'immagine
    Mat dst(temp.size(), CV_8UC3, Scalar(0,0,0));
    Scalar colors[3];
    colors[0] = Scalar(255,0,0);
    colors[1] = Scalar(0,255,0);
    colors[2] = Scalar(0,0,255);
    for (size_t idx = 0; idx < contorni.size(); idx++){
        drawContours(dst,contorni,idx,colors[idx %3]);
        }

    //come produttore
    arg->out->push(dst);
    add++;
    cout<<endl<<"cont ha fatto: "<<add<<endl;
    LeaveCriticalSection(&Lock4);
    }
    cout<<endl<<"Thread (CONTOUR) terminato con "<<add<<" elaborazioni."<<endl;
   _endthread();
}

//main
int main()
{

    coda_concorr<cv::Mat> ingresso;
    coda_concorr<cv::Mat> uscita;

    InitializeConditionVariable(&NonVuoto1);
    InitializeConditionVariable(&NonVuoto2);
    InitializeConditionVariable(&NonVuoto3);
    InitializeConditionVariable(&NonVuoto4);
    InitializeCriticalSection(&Lock1);
    InitializeCriticalSection(&Lock2);
    InitializeCriticalSection(&Lock3);
    InitializeCriticalSection(&Lock4);


    LARGE_INTEGER count1, count2, freq;
    double elapsed;


    Mat temp[10];
    Mat out;

    //dichiarazione code
    Args dati0,dati1,dati2,dati3,dati4;


    //avvio contatori
    QueryPerformanceFrequency(&freq);   
    QueryPerformanceCounter (&count1);

    for(int i=0;i<10;i++){
        temp[i] = imread("C:/OPENCV/Test/imgtest/bird1.jpg",1);
        ingresso.push(temp[i]);
    }

    //next queue pointer
    dati0.in=ingresso;
    dati0.out=&dati1.in;
    dati1.out=&dati2.in;
    dati2.out=&dati3.in;
    dati3.out=&dati4.in;
    dati4.out=&uscita;



    //handle
    HANDLE handle0,handle1,handle2,handle3,handle4;

    //start threads
    handle0 = (HANDLE) _beginthread(puts,0,&dati0);
    handle1 = (HANDLE) _beginthread(grey,0,&dati1);
    handle2 = (HANDLE) _beginthread(soglia,0,&dati2);
    handle3 = (HANDLE) _beginthread(finitura,0,&dati3);
    handle4 = (HANDLE) _beginthread(contorno,0,&dati4);

    cout<<endl<<"..Join dei threads..."<<endl;

    //join
    WaitForSingleObject(handle0,INFINITE);
    WaitForSingleObject(handle1,INFINITE);
    WaitForSingleObject(handle2,INFINITE);
    WaitForSingleObject(handle3,INFINITE);
    WaitForSingleObject(handle4,INFINITE);



    //chiusura contatori
    QueryPerformanceCounter (&count2);

    CloseHandle(handle0);
    CloseHandle(handle1);
    CloseHandle(handle2);
    CloseHandle(handle3);
    CloseHandle(handle4);

    elapsed = (count2.QuadPart - count1.QuadPart) * 1000.0 / freq.QuadPart;


    cout <<endl<<"Tempo di esecuzione approssimativo: " <<elapsed<<" ms."<<endl;
        system("PAUSE");
    return 0;
}

If previuos thread put all it's images on queue, why next thread don't do the same?

I'm on Windows 7 64bit with Visual C++ 2010 OpenCV 2.4.4

Please, help me find where is the problem...

Apart from anything else, do you really need all those CPU-wasting, latency-inducing sleep-polling loops? Use a semaphore to count the queue instead. — Martin James, Jun 18 '13 at 08:47
How can I do with semaphore? What's the improvements? Thanks for your time. — Domenico, Jun 18 '13 at 09:00
Add a semaphore, (MSDN 'CreateSemaphore'), to your queue class, initializing to a count of 0. When pushing, lock your mutex, push, unlock mutex, then signal the semaphore, (MSDN 'ReleaseSemaphore'). When popping, wait on the semaphore first, WaitForSingleObject(queueSema, INFINITE), then lock mutex, pop data, unlock mutex. Such a scheme removes the queue count polling and latency - whenever the semaphore WFSO returns, there will certainly be an object on the queue available. — Martin James, Jun 18 '13 at 09:14
What do you mean with "when WFSO returns"?? Sorry for my insistence, but only recently I have to deal with threads...may you post the code changes you speak of? Thanks in advance for your time. — Domenico, Jun 18 '13 at 14:42
'WFSO' - WaitForSingleObject'. MS API names too much typing :) — Martin James, Jun 20 '13 at 14:36
Ok...thanks Martin, but with semaphore the problem remain (the threads after the first, don't make all works) I think that it's related to while cycle...but How can fix? In what way? I have to modify queue and cycle...but how? — Domenico, Jun 20 '13 at 16:35

score 2 · Answer 1 · edited May 23 '17 at 11:50

It seems what you want to implement is similar to an assembly-line work in a factory where everyone does his job and toss it to the next worker till all the work is done. Please correct me if I am wrong. The idiom on each of your thread function is

void dowork(){
    while(noinput()){
        sleep(0.01);
    }
    while(getSomeInput()){
        processInput();
        queueResult();
    }
    displayAmountOfWorkDone();
}

You are successful in providing mutual exclusion with your mutex. The issue with your design is that once a thread has observed his input queue to be non empty, he consumes all the work and then exits. Due to thread scheduling and to the processing time processInput(), a worker can consume his inputs at a higher rate than the worker before him can produce. For instance taking the queue between init and datit, this is possible:

datit: see 0, sleep
init : see 10 - add 1 
datit: see 1, process
datit: see 0, exit and outputs 1
init : add 2
init : add 3
init : add 4
....
init : add 10

You need to change the design. There should be a different mechanism to signal that the work is over. Right now you are using the amount of inputs that a thread can observe. A quick and dirty fix would be to give to each thread the amount of inputs he is expected to process, and rewrite the algorithm as

void dowork(){
    while(workIsnotDone()){//local test, no lock
        if(getSomeInput()){
             processInput();
             queueResult();
             updateWorkProgress();//local operation
        }
        else{
             sleep(0.01);
        }
    }
    displayAmountOfWorkDone();
}

A better alternative would be to set up the class coda_concorr as a producer-consumer mechanism . For that you can add a condition variable. Each thread is a consumer on one queue and a producer on another. You can also add a field to explicitly specify something like no more inputs. Take a look at this other question on SO

Yes, you understand my problem...it's exactly what happened...how can I solve? How fix this? Thanks in advance for your time. — Domenico, Jun 18 '13 at 08:59
Exactly I need the second option...Every thread is producer for a queue and consumer for another queue...but for doing that is in the question that you've linked, I need boost library for strength? I'm on Visual C++ 2010... — Domenico, Jun 18 '13 at 09:27
you haven't changed the idea behind `puts`, `grey`, `finitura`, `soglia`, `contourno` etc... they still consumes some message and then exit. you need them to stay and look for messages till an external condition is triggered. look again at the function dowork i proposed. — UmNyobe, Jun 21 '13 at 12:43
I have think about condition variables...I have update my code in the OP, but now all threads doing 1 operation (except the thread 0 for clear reasons)...why don't work? Can you look to new code? — Domenico, Jul 03 '13 at 16:40

Visual C++ and concurrency threads..why this happened?

1 Answers1