Multiple C threads not returning correct values












0















I am trying to multiply two matrices using a different thread for each member of the resultant matrix. I have this code:



struct data{
int p;
int linie[20];
int coloana[20];
};

void *func(void *args){

struct data *st = (struct data *) args;
int c = 0;

for(int k = 0; k < st->p; k++){
c += st->linie[k] * st->coloana[k];
}

char *rez = (char*) malloc(5);
sprintf(rez, "%d", c);

return rez;
}

int main(int argc, char *argv){

int n = 2;
int m = 2;

int A[2][2] = {{1, 2},
{4, 5}};

int B[2][2] = {{7, 3},
{7, 5}};

int C[n][m];

char *res[n * m];
char *rez[n * m];

pthread_t threads[n * m];
int count = 0;

for(int i = 0; i < n; i++){
for(int j = 0; j < m; j++){
struct data st;
st.p = 2;
for(int x = 0; x < st.p; x++){
st.linie[x] = A[i][x];
st.coloana[x] = B[x][j];
}

pthread_create(&threads[count], NULL, func, &st);

count++;
}
}

for(int i = 0; i < n * m; i++){
pthread_join(threads[i], (void**) &rez[i]);
printf("%d ", atoi(rez[i]));

}

return 0;
}


But the correct result is never put into rez[i]. For example I get output "63 37 37 37".
The code works perfectly if I don't choose to wait for every thread to finish, i.e. I put that pthread_join right after pthread_create in the nested for loop. What should I do?
Thanks for reading!










share|improve this question

























  • Welcome to Stack Overflow. Please read the About and How to Ask pages soon. Please be careful and make sure the code you post could compile — I fixed a trivial syntax error for you (missing semicolon before end of main()). Also, please indent your code. Copy'n'paste indented code, then select it all and use the {} button above the edit box to indent it by 4 spaces. That should look okay unless you were careless enough to use tabs instead of spaces. Don't use tabs in the code you paste.

    – Jonathan Leffler
    Nov 22 '18 at 0:59


















0















I am trying to multiply two matrices using a different thread for each member of the resultant matrix. I have this code:



struct data{
int p;
int linie[20];
int coloana[20];
};

void *func(void *args){

struct data *st = (struct data *) args;
int c = 0;

for(int k = 0; k < st->p; k++){
c += st->linie[k] * st->coloana[k];
}

char *rez = (char*) malloc(5);
sprintf(rez, "%d", c);

return rez;
}

int main(int argc, char *argv){

int n = 2;
int m = 2;

int A[2][2] = {{1, 2},
{4, 5}};

int B[2][2] = {{7, 3},
{7, 5}};

int C[n][m];

char *res[n * m];
char *rez[n * m];

pthread_t threads[n * m];
int count = 0;

for(int i = 0; i < n; i++){
for(int j = 0; j < m; j++){
struct data st;
st.p = 2;
for(int x = 0; x < st.p; x++){
st.linie[x] = A[i][x];
st.coloana[x] = B[x][j];
}

pthread_create(&threads[count], NULL, func, &st);

count++;
}
}

for(int i = 0; i < n * m; i++){
pthread_join(threads[i], (void**) &rez[i]);
printf("%d ", atoi(rez[i]));

}

return 0;
}


But the correct result is never put into rez[i]. For example I get output "63 37 37 37".
The code works perfectly if I don't choose to wait for every thread to finish, i.e. I put that pthread_join right after pthread_create in the nested for loop. What should I do?
Thanks for reading!










share|improve this question

























  • Welcome to Stack Overflow. Please read the About and How to Ask pages soon. Please be careful and make sure the code you post could compile — I fixed a trivial syntax error for you (missing semicolon before end of main()). Also, please indent your code. Copy'n'paste indented code, then select it all and use the {} button above the edit box to indent it by 4 spaces. That should look okay unless you were careless enough to use tabs instead of spaces. Don't use tabs in the code you paste.

    – Jonathan Leffler
    Nov 22 '18 at 0:59
















0












0








0








I am trying to multiply two matrices using a different thread for each member of the resultant matrix. I have this code:



struct data{
int p;
int linie[20];
int coloana[20];
};

void *func(void *args){

struct data *st = (struct data *) args;
int c = 0;

for(int k = 0; k < st->p; k++){
c += st->linie[k] * st->coloana[k];
}

char *rez = (char*) malloc(5);
sprintf(rez, "%d", c);

return rez;
}

int main(int argc, char *argv){

int n = 2;
int m = 2;

int A[2][2] = {{1, 2},
{4, 5}};

int B[2][2] = {{7, 3},
{7, 5}};

int C[n][m];

char *res[n * m];
char *rez[n * m];

pthread_t threads[n * m];
int count = 0;

for(int i = 0; i < n; i++){
for(int j = 0; j < m; j++){
struct data st;
st.p = 2;
for(int x = 0; x < st.p; x++){
st.linie[x] = A[i][x];
st.coloana[x] = B[x][j];
}

pthread_create(&threads[count], NULL, func, &st);

count++;
}
}

for(int i = 0; i < n * m; i++){
pthread_join(threads[i], (void**) &rez[i]);
printf("%d ", atoi(rez[i]));

}

return 0;
}


But the correct result is never put into rez[i]. For example I get output "63 37 37 37".
The code works perfectly if I don't choose to wait for every thread to finish, i.e. I put that pthread_join right after pthread_create in the nested for loop. What should I do?
Thanks for reading!










share|improve this question
















I am trying to multiply two matrices using a different thread for each member of the resultant matrix. I have this code:



struct data{
int p;
int linie[20];
int coloana[20];
};

void *func(void *args){

struct data *st = (struct data *) args;
int c = 0;

for(int k = 0; k < st->p; k++){
c += st->linie[k] * st->coloana[k];
}

char *rez = (char*) malloc(5);
sprintf(rez, "%d", c);

return rez;
}

int main(int argc, char *argv){

int n = 2;
int m = 2;

int A[2][2] = {{1, 2},
{4, 5}};

int B[2][2] = {{7, 3},
{7, 5}};

int C[n][m];

char *res[n * m];
char *rez[n * m];

pthread_t threads[n * m];
int count = 0;

for(int i = 0; i < n; i++){
for(int j = 0; j < m; j++){
struct data st;
st.p = 2;
for(int x = 0; x < st.p; x++){
st.linie[x] = A[i][x];
st.coloana[x] = B[x][j];
}

pthread_create(&threads[count], NULL, func, &st);

count++;
}
}

for(int i = 0; i < n * m; i++){
pthread_join(threads[i], (void**) &rez[i]);
printf("%d ", atoi(rez[i]));

}

return 0;
}


But the correct result is never put into rez[i]. For example I get output "63 37 37 37".
The code works perfectly if I don't choose to wait for every thread to finish, i.e. I put that pthread_join right after pthread_create in the nested for loop. What should I do?
Thanks for reading!







c multithreading pthreads






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited Nov 22 '18 at 0:57









Jonathan Leffler

573k946881040




573k946881040










asked Nov 22 '18 at 0:51









oftigusoftigus

1




1













  • Welcome to Stack Overflow. Please read the About and How to Ask pages soon. Please be careful and make sure the code you post could compile — I fixed a trivial syntax error for you (missing semicolon before end of main()). Also, please indent your code. Copy'n'paste indented code, then select it all and use the {} button above the edit box to indent it by 4 spaces. That should look okay unless you were careless enough to use tabs instead of spaces. Don't use tabs in the code you paste.

    – Jonathan Leffler
    Nov 22 '18 at 0:59





















  • Welcome to Stack Overflow. Please read the About and How to Ask pages soon. Please be careful and make sure the code you post could compile — I fixed a trivial syntax error for you (missing semicolon before end of main()). Also, please indent your code. Copy'n'paste indented code, then select it all and use the {} button above the edit box to indent it by 4 spaces. That should look okay unless you were careless enough to use tabs instead of spaces. Don't use tabs in the code you paste.

    – Jonathan Leffler
    Nov 22 '18 at 0:59



















Welcome to Stack Overflow. Please read the About and How to Ask pages soon. Please be careful and make sure the code you post could compile — I fixed a trivial syntax error for you (missing semicolon before end of main()). Also, please indent your code. Copy'n'paste indented code, then select it all and use the {} button above the edit box to indent it by 4 spaces. That should look okay unless you were careless enough to use tabs instead of spaces. Don't use tabs in the code you paste.

– Jonathan Leffler
Nov 22 '18 at 0:59







Welcome to Stack Overflow. Please read the About and How to Ask pages soon. Please be careful and make sure the code you post could compile — I fixed a trivial syntax error for you (missing semicolon before end of main()). Also, please indent your code. Copy'n'paste indented code, then select it all and use the {} button above the edit box to indent it by 4 spaces. That should look okay unless you were careless enough to use tabs instead of spaces. Don't use tabs in the code you paste.

– Jonathan Leffler
Nov 22 '18 at 0:59














1 Answer
1






active

oldest

votes


















1














Your first threading problem is here:



for(int i = 0; i < n; i++){
for(int j = 0; j < m; j++){
struct data st;
st.p = 2;
for(int x = 0; x < st.p; x++){
st.linie[x] = A[i][x];
st.coloana[x] = B[x][j];
}
pthread_create(&threads[count], NULL, func, &st);
count++;
}
}


All the threads get passed a pointer to the same variable, &st, which goes out of scope after each call to pthread_create(). You need to ensure that each thread gets its own variable, and that the variable lasts until the thread exits.



To fix this, for example, you could try:



struct data st[n * m];

for (int i = 0; i < n; i++)
{
for (int j = 0; j < m; j++)
{
st[count].p = 2;
for (int x = 0; x < st[count].p; x++)
{
st[count].linie[x] = A[i][x];
st[count].coloana[x] = B[x][j];
}
int rc = pthread_create(&threads[count], NULL, func, &st[count]);
if (rc != 0)
…report pthread creation error…
count++;
}
}


This gives each thread its own struct data to work on, and the structure outlasts the pthread_join() loop.



I'm not completely that it is a good scheme to make one copy of the relevant parts of the two arrays for each thread. It's not too painful at size 2x2, but at 20x20, it begins to be painful. The threads should be told which row and column to process, and should be given pointers to the source matrices, and so on. As long as no thread modifies the source matrices, there isn't a problem reading the data.





Updated answer which replaces the previous invalid code related to pthread_join() (as noted by oftigus in a comment) with this working code. There's a reason I normally test before I post!



On the whole, casts like (void **) should be avoided in the pthread_join() loop. One correct working way to handle this is:



    for (int i = 0; i < n * m; i++)
{
void *vp;
int rc = pthread_join(threads[i], &vp);
if (rc == 0 && vp != NULL)
{
rez[i] = vp;
printf("(%s) %d ", rez[i], atoi(rez[i]));
free(rez[i]);
}
}
putchar('n');


This passes a pointer to a void * variable to pthread_join(). If it finds the information for the requested thread, then pthread_join() makes that void * variable hold the value returned by the thread function. This can then be used as shown — note the error handling (though I note that the example in the POSIX specification for pthread_join()ignores the return value from pthread_join() with a (void) cast on the result).





I don't see where you use res or C.





The result I get is:



(21) 21 (13) 13 (63) 63 (37) 37 


where the value in parentheses is a string and the value outside is converted by atoi(). That looks like the correct answer for multiplying A by B (in that order).



#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

struct data
{
int p;
int linie[20];
int coloana[20];
};

static void *func(void *args)
{
struct data *st = (struct data *)args;
int c = 0;

for (int k = 0; k < st->p; k++)
{
c += st->linie[k] * st->coloana[k];
}

char *rez = (char *)malloc(5);
sprintf(rez, "%d", c);

return rez;
}

int main(void)
{
int n = 2;
int m = 2;
int A[2][2] = {{1, 2}, {4, 5}};
int B[2][2] = {{7, 3}, {7, 5}};
char *rez[n * m];

pthread_t threads[n * m];
int count = 0;
struct data st[n * m];

for (int i = 0; i < n; i++)
{
for (int j = 0; j < m; j++)
{
st[count].p = 2;
for (int x = 0; x < st[count].p; x++)
{
st[count].linie[x] = A[i][x];
st[count].coloana[x] = B[x][j];
}
int rc = pthread_create(&threads[count], NULL, func, &st[count]);
if (rc != 0)
{
fprintf(stderr, "Failed to create thread %d for cell C[%d][%d]n", count, i, j);
exit(1);
}
count++;
}
}

for (int i = 0; i < n * m; i++)
{
void *vp;
int rc = pthread_join(threads[i], &vp);
if (rc == 0 && vp != NULL)
{
rez[i] = vp;
printf("(%s) %d ", rez[i], atoi(rez[i]));
free(rez[i]);
}
}
putchar('n');

return 0;
}





share|improve this answer


























  • Thanks a lot, after changing to an array of structs it works perfectly. I tried to change the pthread_join() too but I got segmentation fault. Also thanks for the welcome, I'll make sure to read up the intro before I ask the next question. Have a nice day!

    – oftigus
    Nov 22 '18 at 2:07













  • You were right! The first code I posted for modifying the pthread_join() loop was wrong — completely and crashingly wrong. I've updated it with correct code that doesn't crash and does produce the correct answer without the bludgeoning (void **) cast — it needs no cast, in fact.

    – Jonathan Leffler
    Nov 22 '18 at 4:44












Your Answer






StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");

StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "1"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});

function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: true,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: 10,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});


}
});














draft saved

draft discarded


















StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53422467%2fmultiple-c-threads-not-returning-correct-values%23new-answer', 'question_page');
}
);

Post as a guest















Required, but never shown

























1 Answer
1






active

oldest

votes








1 Answer
1






active

oldest

votes









active

oldest

votes






active

oldest

votes









1














Your first threading problem is here:



for(int i = 0; i < n; i++){
for(int j = 0; j < m; j++){
struct data st;
st.p = 2;
for(int x = 0; x < st.p; x++){
st.linie[x] = A[i][x];
st.coloana[x] = B[x][j];
}
pthread_create(&threads[count], NULL, func, &st);
count++;
}
}


All the threads get passed a pointer to the same variable, &st, which goes out of scope after each call to pthread_create(). You need to ensure that each thread gets its own variable, and that the variable lasts until the thread exits.



To fix this, for example, you could try:



struct data st[n * m];

for (int i = 0; i < n; i++)
{
for (int j = 0; j < m; j++)
{
st[count].p = 2;
for (int x = 0; x < st[count].p; x++)
{
st[count].linie[x] = A[i][x];
st[count].coloana[x] = B[x][j];
}
int rc = pthread_create(&threads[count], NULL, func, &st[count]);
if (rc != 0)
…report pthread creation error…
count++;
}
}


This gives each thread its own struct data to work on, and the structure outlasts the pthread_join() loop.



I'm not completely that it is a good scheme to make one copy of the relevant parts of the two arrays for each thread. It's not too painful at size 2x2, but at 20x20, it begins to be painful. The threads should be told which row and column to process, and should be given pointers to the source matrices, and so on. As long as no thread modifies the source matrices, there isn't a problem reading the data.





Updated answer which replaces the previous invalid code related to pthread_join() (as noted by oftigus in a comment) with this working code. There's a reason I normally test before I post!



On the whole, casts like (void **) should be avoided in the pthread_join() loop. One correct working way to handle this is:



    for (int i = 0; i < n * m; i++)
{
void *vp;
int rc = pthread_join(threads[i], &vp);
if (rc == 0 && vp != NULL)
{
rez[i] = vp;
printf("(%s) %d ", rez[i], atoi(rez[i]));
free(rez[i]);
}
}
putchar('n');


This passes a pointer to a void * variable to pthread_join(). If it finds the information for the requested thread, then pthread_join() makes that void * variable hold the value returned by the thread function. This can then be used as shown — note the error handling (though I note that the example in the POSIX specification for pthread_join()ignores the return value from pthread_join() with a (void) cast on the result).





I don't see where you use res or C.





The result I get is:



(21) 21 (13) 13 (63) 63 (37) 37 


where the value in parentheses is a string and the value outside is converted by atoi(). That looks like the correct answer for multiplying A by B (in that order).



#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

struct data
{
int p;
int linie[20];
int coloana[20];
};

static void *func(void *args)
{
struct data *st = (struct data *)args;
int c = 0;

for (int k = 0; k < st->p; k++)
{
c += st->linie[k] * st->coloana[k];
}

char *rez = (char *)malloc(5);
sprintf(rez, "%d", c);

return rez;
}

int main(void)
{
int n = 2;
int m = 2;
int A[2][2] = {{1, 2}, {4, 5}};
int B[2][2] = {{7, 3}, {7, 5}};
char *rez[n * m];

pthread_t threads[n * m];
int count = 0;
struct data st[n * m];

for (int i = 0; i < n; i++)
{
for (int j = 0; j < m; j++)
{
st[count].p = 2;
for (int x = 0; x < st[count].p; x++)
{
st[count].linie[x] = A[i][x];
st[count].coloana[x] = B[x][j];
}
int rc = pthread_create(&threads[count], NULL, func, &st[count]);
if (rc != 0)
{
fprintf(stderr, "Failed to create thread %d for cell C[%d][%d]n", count, i, j);
exit(1);
}
count++;
}
}

for (int i = 0; i < n * m; i++)
{
void *vp;
int rc = pthread_join(threads[i], &vp);
if (rc == 0 && vp != NULL)
{
rez[i] = vp;
printf("(%s) %d ", rez[i], atoi(rez[i]));
free(rez[i]);
}
}
putchar('n');

return 0;
}





share|improve this answer


























  • Thanks a lot, after changing to an array of structs it works perfectly. I tried to change the pthread_join() too but I got segmentation fault. Also thanks for the welcome, I'll make sure to read up the intro before I ask the next question. Have a nice day!

    – oftigus
    Nov 22 '18 at 2:07













  • You were right! The first code I posted for modifying the pthread_join() loop was wrong — completely and crashingly wrong. I've updated it with correct code that doesn't crash and does produce the correct answer without the bludgeoning (void **) cast — it needs no cast, in fact.

    – Jonathan Leffler
    Nov 22 '18 at 4:44
















1














Your first threading problem is here:



for(int i = 0; i < n; i++){
for(int j = 0; j < m; j++){
struct data st;
st.p = 2;
for(int x = 0; x < st.p; x++){
st.linie[x] = A[i][x];
st.coloana[x] = B[x][j];
}
pthread_create(&threads[count], NULL, func, &st);
count++;
}
}


All the threads get passed a pointer to the same variable, &st, which goes out of scope after each call to pthread_create(). You need to ensure that each thread gets its own variable, and that the variable lasts until the thread exits.



To fix this, for example, you could try:



struct data st[n * m];

for (int i = 0; i < n; i++)
{
for (int j = 0; j < m; j++)
{
st[count].p = 2;
for (int x = 0; x < st[count].p; x++)
{
st[count].linie[x] = A[i][x];
st[count].coloana[x] = B[x][j];
}
int rc = pthread_create(&threads[count], NULL, func, &st[count]);
if (rc != 0)
…report pthread creation error…
count++;
}
}


This gives each thread its own struct data to work on, and the structure outlasts the pthread_join() loop.



I'm not completely that it is a good scheme to make one copy of the relevant parts of the two arrays for each thread. It's not too painful at size 2x2, but at 20x20, it begins to be painful. The threads should be told which row and column to process, and should be given pointers to the source matrices, and so on. As long as no thread modifies the source matrices, there isn't a problem reading the data.





Updated answer which replaces the previous invalid code related to pthread_join() (as noted by oftigus in a comment) with this working code. There's a reason I normally test before I post!



On the whole, casts like (void **) should be avoided in the pthread_join() loop. One correct working way to handle this is:



    for (int i = 0; i < n * m; i++)
{
void *vp;
int rc = pthread_join(threads[i], &vp);
if (rc == 0 && vp != NULL)
{
rez[i] = vp;
printf("(%s) %d ", rez[i], atoi(rez[i]));
free(rez[i]);
}
}
putchar('n');


This passes a pointer to a void * variable to pthread_join(). If it finds the information for the requested thread, then pthread_join() makes that void * variable hold the value returned by the thread function. This can then be used as shown — note the error handling (though I note that the example in the POSIX specification for pthread_join()ignores the return value from pthread_join() with a (void) cast on the result).





I don't see where you use res or C.





The result I get is:



(21) 21 (13) 13 (63) 63 (37) 37 


where the value in parentheses is a string and the value outside is converted by atoi(). That looks like the correct answer for multiplying A by B (in that order).



#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

struct data
{
int p;
int linie[20];
int coloana[20];
};

static void *func(void *args)
{
struct data *st = (struct data *)args;
int c = 0;

for (int k = 0; k < st->p; k++)
{
c += st->linie[k] * st->coloana[k];
}

char *rez = (char *)malloc(5);
sprintf(rez, "%d", c);

return rez;
}

int main(void)
{
int n = 2;
int m = 2;
int A[2][2] = {{1, 2}, {4, 5}};
int B[2][2] = {{7, 3}, {7, 5}};
char *rez[n * m];

pthread_t threads[n * m];
int count = 0;
struct data st[n * m];

for (int i = 0; i < n; i++)
{
for (int j = 0; j < m; j++)
{
st[count].p = 2;
for (int x = 0; x < st[count].p; x++)
{
st[count].linie[x] = A[i][x];
st[count].coloana[x] = B[x][j];
}
int rc = pthread_create(&threads[count], NULL, func, &st[count]);
if (rc != 0)
{
fprintf(stderr, "Failed to create thread %d for cell C[%d][%d]n", count, i, j);
exit(1);
}
count++;
}
}

for (int i = 0; i < n * m; i++)
{
void *vp;
int rc = pthread_join(threads[i], &vp);
if (rc == 0 && vp != NULL)
{
rez[i] = vp;
printf("(%s) %d ", rez[i], atoi(rez[i]));
free(rez[i]);
}
}
putchar('n');

return 0;
}





share|improve this answer


























  • Thanks a lot, after changing to an array of structs it works perfectly. I tried to change the pthread_join() too but I got segmentation fault. Also thanks for the welcome, I'll make sure to read up the intro before I ask the next question. Have a nice day!

    – oftigus
    Nov 22 '18 at 2:07













  • You were right! The first code I posted for modifying the pthread_join() loop was wrong — completely and crashingly wrong. I've updated it with correct code that doesn't crash and does produce the correct answer without the bludgeoning (void **) cast — it needs no cast, in fact.

    – Jonathan Leffler
    Nov 22 '18 at 4:44














1












1








1







Your first threading problem is here:



for(int i = 0; i < n; i++){
for(int j = 0; j < m; j++){
struct data st;
st.p = 2;
for(int x = 0; x < st.p; x++){
st.linie[x] = A[i][x];
st.coloana[x] = B[x][j];
}
pthread_create(&threads[count], NULL, func, &st);
count++;
}
}


All the threads get passed a pointer to the same variable, &st, which goes out of scope after each call to pthread_create(). You need to ensure that each thread gets its own variable, and that the variable lasts until the thread exits.



To fix this, for example, you could try:



struct data st[n * m];

for (int i = 0; i < n; i++)
{
for (int j = 0; j < m; j++)
{
st[count].p = 2;
for (int x = 0; x < st[count].p; x++)
{
st[count].linie[x] = A[i][x];
st[count].coloana[x] = B[x][j];
}
int rc = pthread_create(&threads[count], NULL, func, &st[count]);
if (rc != 0)
…report pthread creation error…
count++;
}
}


This gives each thread its own struct data to work on, and the structure outlasts the pthread_join() loop.



I'm not completely that it is a good scheme to make one copy of the relevant parts of the two arrays for each thread. It's not too painful at size 2x2, but at 20x20, it begins to be painful. The threads should be told which row and column to process, and should be given pointers to the source matrices, and so on. As long as no thread modifies the source matrices, there isn't a problem reading the data.





Updated answer which replaces the previous invalid code related to pthread_join() (as noted by oftigus in a comment) with this working code. There's a reason I normally test before I post!



On the whole, casts like (void **) should be avoided in the pthread_join() loop. One correct working way to handle this is:



    for (int i = 0; i < n * m; i++)
{
void *vp;
int rc = pthread_join(threads[i], &vp);
if (rc == 0 && vp != NULL)
{
rez[i] = vp;
printf("(%s) %d ", rez[i], atoi(rez[i]));
free(rez[i]);
}
}
putchar('n');


This passes a pointer to a void * variable to pthread_join(). If it finds the information for the requested thread, then pthread_join() makes that void * variable hold the value returned by the thread function. This can then be used as shown — note the error handling (though I note that the example in the POSIX specification for pthread_join()ignores the return value from pthread_join() with a (void) cast on the result).





I don't see where you use res or C.





The result I get is:



(21) 21 (13) 13 (63) 63 (37) 37 


where the value in parentheses is a string and the value outside is converted by atoi(). That looks like the correct answer for multiplying A by B (in that order).



#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

struct data
{
int p;
int linie[20];
int coloana[20];
};

static void *func(void *args)
{
struct data *st = (struct data *)args;
int c = 0;

for (int k = 0; k < st->p; k++)
{
c += st->linie[k] * st->coloana[k];
}

char *rez = (char *)malloc(5);
sprintf(rez, "%d", c);

return rez;
}

int main(void)
{
int n = 2;
int m = 2;
int A[2][2] = {{1, 2}, {4, 5}};
int B[2][2] = {{7, 3}, {7, 5}};
char *rez[n * m];

pthread_t threads[n * m];
int count = 0;
struct data st[n * m];

for (int i = 0; i < n; i++)
{
for (int j = 0; j < m; j++)
{
st[count].p = 2;
for (int x = 0; x < st[count].p; x++)
{
st[count].linie[x] = A[i][x];
st[count].coloana[x] = B[x][j];
}
int rc = pthread_create(&threads[count], NULL, func, &st[count]);
if (rc != 0)
{
fprintf(stderr, "Failed to create thread %d for cell C[%d][%d]n", count, i, j);
exit(1);
}
count++;
}
}

for (int i = 0; i < n * m; i++)
{
void *vp;
int rc = pthread_join(threads[i], &vp);
if (rc == 0 && vp != NULL)
{
rez[i] = vp;
printf("(%s) %d ", rez[i], atoi(rez[i]));
free(rez[i]);
}
}
putchar('n');

return 0;
}





share|improve this answer















Your first threading problem is here:



for(int i = 0; i < n; i++){
for(int j = 0; j < m; j++){
struct data st;
st.p = 2;
for(int x = 0; x < st.p; x++){
st.linie[x] = A[i][x];
st.coloana[x] = B[x][j];
}
pthread_create(&threads[count], NULL, func, &st);
count++;
}
}


All the threads get passed a pointer to the same variable, &st, which goes out of scope after each call to pthread_create(). You need to ensure that each thread gets its own variable, and that the variable lasts until the thread exits.



To fix this, for example, you could try:



struct data st[n * m];

for (int i = 0; i < n; i++)
{
for (int j = 0; j < m; j++)
{
st[count].p = 2;
for (int x = 0; x < st[count].p; x++)
{
st[count].linie[x] = A[i][x];
st[count].coloana[x] = B[x][j];
}
int rc = pthread_create(&threads[count], NULL, func, &st[count]);
if (rc != 0)
…report pthread creation error…
count++;
}
}


This gives each thread its own struct data to work on, and the structure outlasts the pthread_join() loop.



I'm not completely that it is a good scheme to make one copy of the relevant parts of the two arrays for each thread. It's not too painful at size 2x2, but at 20x20, it begins to be painful. The threads should be told which row and column to process, and should be given pointers to the source matrices, and so on. As long as no thread modifies the source matrices, there isn't a problem reading the data.





Updated answer which replaces the previous invalid code related to pthread_join() (as noted by oftigus in a comment) with this working code. There's a reason I normally test before I post!



On the whole, casts like (void **) should be avoided in the pthread_join() loop. One correct working way to handle this is:



    for (int i = 0; i < n * m; i++)
{
void *vp;
int rc = pthread_join(threads[i], &vp);
if (rc == 0 && vp != NULL)
{
rez[i] = vp;
printf("(%s) %d ", rez[i], atoi(rez[i]));
free(rez[i]);
}
}
putchar('n');


This passes a pointer to a void * variable to pthread_join(). If it finds the information for the requested thread, then pthread_join() makes that void * variable hold the value returned by the thread function. This can then be used as shown — note the error handling (though I note that the example in the POSIX specification for pthread_join()ignores the return value from pthread_join() with a (void) cast on the result).





I don't see where you use res or C.





The result I get is:



(21) 21 (13) 13 (63) 63 (37) 37 


where the value in parentheses is a string and the value outside is converted by atoi(). That looks like the correct answer for multiplying A by B (in that order).



#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

struct data
{
int p;
int linie[20];
int coloana[20];
};

static void *func(void *args)
{
struct data *st = (struct data *)args;
int c = 0;

for (int k = 0; k < st->p; k++)
{
c += st->linie[k] * st->coloana[k];
}

char *rez = (char *)malloc(5);
sprintf(rez, "%d", c);

return rez;
}

int main(void)
{
int n = 2;
int m = 2;
int A[2][2] = {{1, 2}, {4, 5}};
int B[2][2] = {{7, 3}, {7, 5}};
char *rez[n * m];

pthread_t threads[n * m];
int count = 0;
struct data st[n * m];

for (int i = 0; i < n; i++)
{
for (int j = 0; j < m; j++)
{
st[count].p = 2;
for (int x = 0; x < st[count].p; x++)
{
st[count].linie[x] = A[i][x];
st[count].coloana[x] = B[x][j];
}
int rc = pthread_create(&threads[count], NULL, func, &st[count]);
if (rc != 0)
{
fprintf(stderr, "Failed to create thread %d for cell C[%d][%d]n", count, i, j);
exit(1);
}
count++;
}
}

for (int i = 0; i < n * m; i++)
{
void *vp;
int rc = pthread_join(threads[i], &vp);
if (rc == 0 && vp != NULL)
{
rez[i] = vp;
printf("(%s) %d ", rez[i], atoi(rez[i]));
free(rez[i]);
}
}
putchar('n');

return 0;
}






share|improve this answer














share|improve this answer



share|improve this answer








edited Nov 22 '18 at 4:42

























answered Nov 22 '18 at 1:02









Jonathan LefflerJonathan Leffler

573k946881040




573k946881040













  • Thanks a lot, after changing to an array of structs it works perfectly. I tried to change the pthread_join() too but I got segmentation fault. Also thanks for the welcome, I'll make sure to read up the intro before I ask the next question. Have a nice day!

    – oftigus
    Nov 22 '18 at 2:07













  • You were right! The first code I posted for modifying the pthread_join() loop was wrong — completely and crashingly wrong. I've updated it with correct code that doesn't crash and does produce the correct answer without the bludgeoning (void **) cast — it needs no cast, in fact.

    – Jonathan Leffler
    Nov 22 '18 at 4:44



















  • Thanks a lot, after changing to an array of structs it works perfectly. I tried to change the pthread_join() too but I got segmentation fault. Also thanks for the welcome, I'll make sure to read up the intro before I ask the next question. Have a nice day!

    – oftigus
    Nov 22 '18 at 2:07













  • You were right! The first code I posted for modifying the pthread_join() loop was wrong — completely and crashingly wrong. I've updated it with correct code that doesn't crash and does produce the correct answer without the bludgeoning (void **) cast — it needs no cast, in fact.

    – Jonathan Leffler
    Nov 22 '18 at 4:44

















Thanks a lot, after changing to an array of structs it works perfectly. I tried to change the pthread_join() too but I got segmentation fault. Also thanks for the welcome, I'll make sure to read up the intro before I ask the next question. Have a nice day!

– oftigus
Nov 22 '18 at 2:07







Thanks a lot, after changing to an array of structs it works perfectly. I tried to change the pthread_join() too but I got segmentation fault. Also thanks for the welcome, I'll make sure to read up the intro before I ask the next question. Have a nice day!

– oftigus
Nov 22 '18 at 2:07















You were right! The first code I posted for modifying the pthread_join() loop was wrong — completely and crashingly wrong. I've updated it with correct code that doesn't crash and does produce the correct answer without the bludgeoning (void **) cast — it needs no cast, in fact.

– Jonathan Leffler
Nov 22 '18 at 4:44





You were right! The first code I posted for modifying the pthread_join() loop was wrong — completely and crashingly wrong. I've updated it with correct code that doesn't crash and does produce the correct answer without the bludgeoning (void **) cast — it needs no cast, in fact.

– Jonathan Leffler
Nov 22 '18 at 4:44




















draft saved

draft discarded




















































Thanks for contributing an answer to Stack Overflow!


  • Please be sure to answer the question. Provide details and share your research!

But avoid



  • Asking for help, clarification, or responding to other answers.

  • Making statements based on opinion; back them up with references or personal experience.


To learn more, see our tips on writing great answers.




draft saved


draft discarded














StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53422467%2fmultiple-c-threads-not-returning-correct-values%23new-answer', 'question_page');
}
);

Post as a guest















Required, but never shown





















































Required, but never shown














Required, but never shown












Required, but never shown







Required, but never shown

































Required, but never shown














Required, but never shown












Required, but never shown







Required, but never shown







Popular posts from this blog

Biblatex bibliography style without URLs when DOI exists (in Overleaf with Zotero bibliography)

ComboBox Display Member on multiple fields

Is it possible to collect Nectar points via Trainline?