Multiple C threads not returning correct values
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
add a comment |
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
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 ofmain()
). 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
add a comment |
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
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
c multithreading pthreads
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 ofmain()
). 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
add a comment |
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 ofmain()
). 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
add a comment |
1 Answer
1
active
oldest
votes
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;
}
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 thepthread_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
add a comment |
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
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
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
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;
}
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 thepthread_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
add a comment |
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;
}
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 thepthread_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
add a comment |
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;
}
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;
}
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 thepthread_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
add a comment |
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 thepthread_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
add a comment |
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.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
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
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
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
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