CODEWITHSUNDEEP
cpthreadsmax
Marios Koni
Marios Koni

Reputation: 143

Second pthread won't enter second loop

I'm facing a problem with my threads. I'm trying to find the max element of the main diagonal in a 2-dimensional array. The first pthread will do its thing and find the max element. But, when the second and thrird and so on array "joins the party" they will not enter the second while. My function looks like this:

void* findMax() {
    //int t = 1;

    //*arr.times = 0;

    for (*arr.countI = 0; *arr.countI < *arr.l; (*arr.countI)++) {
        for (*arr.countJ = 0; *arr.countJ < *arr.l; (*arr.countJ)++) {
            printf("%d ", arr.a[*arr.countI][*arr.countJ]);
            if (*arr.l - *arr.countJ == 1)
                printf("\n");
        }
    }

    *arr.countI = 0;
    *arr.countJ = 0;

    while (*arr.countI != *arr.l) {
        while (*arr.countJ != *arr.l) {
            if (*arr.countI == *arr.countJ) {
                if (abs(arr.a[*arr.countI][*arr.countJ]) > *arr.max)
                    *arr.max = abs(arr.a[*arr.countI][*arr.countJ]);
            }

            ++(*arr.countJ);
        }

        ++(*arr.countI);

        /*if (++(*arr.times) == 1) {
            *arr.times = 0;
            break;
        }*/
    }

    printf("max = %d\n", *arr.max);
}

So if I enter this array

 4 1 1
 1 6 1
 1 1 3

the max will equal 4 which is not right of course.

arr is a struct (global) that has everything I need; arr.a is the array.

I have messed with it a lot over the last couple of days but with no luck...

Upvotes: 0

Views: 87

Answers (1)

Jonathan Leffler
Jonathan Leffler

Reputation: 753665

As previously noted in comments, you have no mutual exclusion, so your threads are all busily manipulating elements of the global array without regard to what other threads are doing.

All those *arr.countI etc references are ugly. Where is the mutual exclusion that makes sure that no thread is modifying anything inside arr while another thread is also modifying it? You have *arr.countI = 0;, for example, but no visible mutual exclusion. You have *arr.max = abs(arr.a[*arr.countI][*arr.countJ]); and no mutual exclusion. You have ++(*arr.countJ); and no mutual exclusion. None of the threads has the first clue about what's going on. Maybe you're lucky and *arr.countI == *arr.l so they don't enter the loop. But you've no concurrency control so everything is guesswork.

I think you should stop using countI and countJ from the global data, and also arr.max most of the time. Those should be replaced by per-thread local variables (int i; int j; int max;). If you're scanning down the main diagonal, you don't need nested loops — you can simply step down the diagonal. Your code is very complex for a very simple job. You still need a mutex to allow *arr.max to be set reliably. arr.a and *arr.l are 'OK' (but why is *arr.l a pointer?) because you don't modify those. You promise to return a void * and return nothing. Lying to your compiler is a Bad Idea™.

If your threads store anything into your global structure (arr), you need to make sure there is mutual exclusion in place. I assume you have or add a member mtx which is a pointer to a pthread_mutex_t that is initialized before the thread functions are invoked. I'm also assuming your findMax() function is called from pthread_create() (and passed to it as a function pointer).

You could use much simpler code for the task, like this:

void *findMax(void *arg)
{
    assert(arg == 0);

    for (int i = 0; i < *arr.l; i++) {
        for (int j = 0; j < *arr.l; j++) {
            printf("%d ", arr.a[i][j]);
        }
        printf("\n");
    }

    int max = abs(arr.a[0][0]);
    for (int i = 1; i < *arr.l; i++)
    {
        int absval = abs(arr.a[i][i]);
        if (absval > max)
            max = absval;
    }

    if (pthread_mutex_lock(arr.mtx) == 0)
    {
        *arr.max = max;
        pthread_mutex_unlock(arr.mtx);
    }

    printf("max = %d\n", *arr.max);
    return 0;
}

Warning: uncompiled code!

Upvotes: 1

Related Questions