Why the output goes wrong when using pthread_join?

Question

I am still learning about threads and I was trying to solve this problem in my code, when I am putting the pthread_join(thread[i],NULL) outside the loop that is creating the threads it always gives me wrong output and Thread with ID = 0 will not work(call the median func) and the last thread will work two times, for better understanding see the output below:

ThreadID= 0, startRow= 0, endRow= 0      // first thread doesn't call the median func
ThreadID= 1, startRow= 1, endRow= 1
ThreadID 1 numOfBright 0 numOfDark 1 numOfNormal 4
ThreadID= 2, startRow= 2, endRow= 2
ThreadID 2 numOfBright 0 numOfDark 1 numOfNormal 4
ThreadID= 3, startRow= 3, endRow= 3
ThreadID 3 numOfBright 0 numOfDark 0 numOfNormal 5
ThreadID= 4, startRow= 4, endRow= 4
ThreadID 4 numOfBright 0 numOfDark 5 numOfNormal 0  
ThreadID 4 numOfBright 0 numOfDark 5 numOfNormal 0 // last thread is calling the median func two times.

This is the part of the code that prints the start and end row of each thread.


    pthread_t* threads = new pthread_t[num_threads];
    struct Th_Range* RANGE = (struct Th_Range*)malloc(sizeof(struct Th_Range*));
    int thread_status;
    RANGE->SizeOfImage = r; // 2d array with size (n*n) so rows(r) = columns(c) 
    if (n == num_threads) {  //rows = num of threads then every thread will work in a single row
        for (int i = 0; i < num_threads; i++) {
            RANGE->ThreadId = i;
            RANGE->StartRow = RANGE->EndRow = i;
            cout << "ThreadID= " << i << ", startRow= " << RANGE->StartRow << ", endRow= " << RANGE->EndRow << endl;
            thread_status = pthread_create(&threads[i], NULL, Median, RANGE);
            if (thread_status) 
        exit(-1);
                                             } //for loop ends here
           for (int i = 0; i < num_threads; i++)
           pthread_join(threads[i],NULL);
  } //end of if statement

Here is the part of the code if needed with the median function and the above if statement.

#include <iostream>
#include <bits/stdc++.h>
#include <pthread.h>
 
pthread_mutex_t Lock;
pthread_mutex_t Pixels;
pthread_mutex_t Pixels2;
using namespace std;
 
int numOfBright, numOfDark, numOfNormal;
int** Oimage, ** Fimage; //original and filtered image
struct Th_Range {
    int SizeOfImage;
    int StartRow;
    int EndRow;
    int ThreadId;
};
void* Median(void* par)
{
    struct Th_Range* Num = (struct Th_Range*)par;
    int StartRow = Num->StartRow;
    int EndRow = Num->EndRow;
    int Size = Num->SizeOfImage;    
    int Neighbour[9] = { 0 };
    int dark = 0, bright = 0, normal = 0;
    if (EndRow == StartRow)   
        EndRow += 2;
    else
        EndRow++;
    for (int i = StartRow +1; i < EndRow ; i++)
    {
        for (int j = 1; j < Size - 1; j++)
        {
            Neighbour[0] = Oimage[i - 1][j - 1];
            Neighbour[1] = Oimage[i - 1][j];
            Neighbour[2] = Oimage[i - 1][j + 1];
            Neighbour[3] = Oimage[i][j - 1];
            Neighbour[4] = Oimage[i][j];
            Neighbour[5] = Oimage[i][j + 1];
            Neighbour[6] = Oimage[i + 1][j - 1];
            Neighbour[7] = Oimage[i + 1][j];
            Neighbour[8] = Oimage[i + 1][j + 1];
 
            pthread_mutex_lock(&Pixels); //it can be moved only to lock the Fimage and the numOfBright or any other global variables
            sort(Neighbour, Neighbour + 9);
            Fimage[i][j] = Neighbour[4];
            if (Neighbour[4] > 200) {
                bright++;
                numOfBright++;
            }
            else if (Neighbour[4] < 50) {
                dark++;
                numOfDark++;
            }
            else {
                normal++;
                numOfNormal++;
            }
            pthread_mutex_unlock(&Pixels);
        }
    }
    pthread_mutex_lock(&Pixels2);  //when I try to remove this lock the output gets interrupted
    cout << "ThreadID " << Num->ThreadId << " numOfBright " << bright << " numOfDark " << dark << " numOfNormal " << normal<<endl;
    pthread_mutex_unlock(&Pixels2);
    pthread_exit(NULL);
}
 
int main(int argc, char* argv[])
{
    int num_threads, n, r, c;  // n is the size of the matrix r and c are rows and columns
 
    numOfNormal = numOfDark = numOfBright = 0;
    if (argc >= 2)
        num_threads = atoi(argv[1]);
    else
        exit(-1);
 
    ifstream cin("input.txt");
 
 
    cin >> n;
    r = c = n + 2;
 
    Oimage = new int* [r]();
    Fimage = new int* [r]();
 
    for (int i = 0; i < c; i++)
    {
        Oimage[i] = new int[c]();
        Fimage[i] = new int[c]();
    }
 
    for (int i = 1; i < r - 1; i++)  
        for (int j = 1; j < c - 1; j++)
            cin >> Oimage[i][j];
 
    
 
  
    pthread_t* threads = new pthread_t[num_threads];
    struct Th_Range* RANGE = (struct Th_Range*)malloc(sizeof(struct Th_Range*));
 
    RANGE->SizeOfImage = r;
    if (n == num_threads) {  //rows = num of threads then every thread will work in a single row
        //n+2 
    int thread_status;
        for (int i = 0; i < num_threads; i++) {
            RANGE->ThreadId = i;
            RANGE->StartRow = RANGE->EndRow = i;
         //   pthread_mutex_lock(&Lock);
            cout << "ThreadID= " << i << ", startRow= " << RANGE->StartRow << ", endRow= " << RANGE->EndRow << endl;
            
            thread_status = pthread_create(&threads[i], NULL, Median, RANGE);
        
            if (thread_status) 
        exit(-1);
    }

  }

I tried to move pthread_join inside the loop of pthread_create it gives a correct output but of course it is a wrong solution. I have no idea what to do next. Thanks in advance

For one thing, you are using the same `struct Th_Range` object to provide working ranges to all your threads. I warned you about exactly this in [my answer to your previous question](https://stackoverflow.com/a/74891969/2402272). — John Bollinger, Dec 25 '22 at 14:35
But only sort of, because you are allocating that dynamically via `malloc()`, and not specifying a large enough size. — John Bollinger, Dec 25 '22 at 14:36
@JohnBollinger I have moved struct Th_Range* RANGE = (struct Th_Range*)malloc(sizeof(struct Th_Range*)); inside every if statement but the same problem I don't know if I understand you correctly but can you show me an example of what you are saying — NET_MASKK9908, Dec 25 '22 at 15:25
Every thread needs its own. Moving the `malloc()` inside the `if` does not itself achieve that unless you move it all the way inside the `for`. (The one in which`pthread_create()` is called, of course.) And again, you are allocating the wrong size. You need space for a `struct Th_Range`, but you are allocating only enough for a pointer. — John Bollinger, Dec 25 '22 at 15:29
It worked, but regarding the allocation size how can I determine it ? this is the first time I deal with threads and memory allocation so I am not very good at it sorry for asking. @JohnBollinger — NET_MASKK9908, Dec 25 '22 at 16:09
If you are allocating space via `malloc()` and assigning it to a `Thing *`, then you presumably want space for one or more `Thing`s. That would be in the amount of `n * sizeof(Thing)` bytes for some integer `n`. If the name of the variable to which you are assigning the `malloc` result is `my_thing`, then you can also spell that `n * sizeof(*my_thing)`, which is robust against changes to the type of `my_thing`. But it would be more idiomatic for C++ to use the `new` or new[] operator, which computes the needed size automatically. — John Bollinger, Dec 25 '22 at 22:12

score -1 · Answer 1 · answered Dec 25 '22 at 14:41

-1

Maybe you should use #include or (using namespace sff) it must work

answered Dec 25 '22 at 14:41

the godfather

1

1

How does this answer relate to the question post? Proper #include and namespace could solve **compiler** errors, but the question post is about how the code **runs**. – Tsyvarev Dec 25 '22 at 15:26
Your answer could be improved with additional supporting information. Please [edit] to add further details, such as citations or documentation, so that others can confirm that your answer is correct. You can find more information on how to write good answers [in the help center](/help/how-to-answer). – Community Dec 26 '22 at 07:44

Why the output goes wrong when using pthread_join?

1 Answers1