Reputation: 21
#include <pthread.h>
#include <stdio.h>
#define WIDTH 1000
#define HEIGHT 1000
typedef struct s_pair
{
int x;
int y;
} t_pair;
char pixels[HEIGHT][WIDTH];
void *add_to_global(void *param)
{
t_pair *input;
input = (t_pair *)param;
if (!(input->x % 2))
pixels[input->y][input->x] = '.';
else if (!(input->x % 3))
pixels[input->y][input->x] = '#';
else
pixels[input->y][input->x] = ' ';
return (NULL);
}
void thread_runner(int x_start, int x_end, int y_start, int y_end)
{
pthread_t workers[10];
t_pair inputs[10];
int i, x, y;
y = y_start - 1;
while (++y < y_end)
{
x = x_start - 1;
i = -1;
while (x < x_end)
{
if (++i < 10)
{
inputs[i] = (t_pair){++x, y};
pthread_create(&workers[i], NULL, add_to_global, (void *)&inputs[i]);
}
else
while (--i > -1)
pthread_join(workers[i], NULL);
}
while (--i > -1)
pthread_join(workers[i], NULL);
printf("%s\n", pixels[y]); // if this is removed, this program will never finish
}
}
int main()
{
thread_runner(0, WIDTH, 0, HEIGHT);
return (0);
}
So originally while trying to make a fractal visualizer I ran into this problem where I could use this method of running ten threads at a time only if I drew the image on the screen while doing it. If I instead let the program run by itself, it became completely unresponsive and my mac began making a disturbing noise. I'm still new to threads so this might be something very simple I've misunderstood about them.
This program is my attempt at reproducing the phenomenon without the need of all the fractal nonsense. It is a completely meaningless program that simply tells you whether the x-coordinate of a point is divisible by 2 or 3 and prints certain characters to the terminal accordingly.
If you remove the printf which prints each line y after it has processed each point x on that line, the program will become unresponsive. However, running the program with this periodic printing makes it work. This just seems like some programming-demons are playing tricks with me and making the problem undebuggable.
Why is this problem fixed by a seemingly meaningless printf? Why is my mac making a disturbing sound when I run it without this printf?
Upvotes: 2
Views: 328
Reputation: 224377
You're reading / writing past the end of your arrays.
x = x_start - 1;
i = -1;
while (x < x_end)
{
if (++i < 10)
{
inputs[i] = (t_pair){++x, y};
pthread_create(&workers[i], NULL, add_to_global, (void *)&inputs[i]);
}
else
while (--i > -1)
pthread_join(workers[i], NULL);
}
In this block, the last iteration of the outer loop has x
equal to x_end-1
. Then when you create the input, x
gets incremented so that inputs[i].x
is x_end
. This value is then used to write past the end of one of the array dimensions.
This triggers undefined behavior which you see as a call to printf
causing / preventing an infinite loop.
Rather than starting your loops with -1 and incrementing either in the loop condition or in the middle of the body, start at 0 and increment at the end.
void thread_runner(int x_start, int x_end, int y_start, int y_end)
{
pthread_t workers[10];
t_pair inputs[10];
int i, x, y;
y = 0;
while (y < y_end)
{
i = 0;
x = 0;
while (x < x_end)
{
if (i < 10)
{
inputs[i] = (t_pair){x, y};
pthread_create(&workers[i], NULL, add_to_global, &inputs[i]);
i++;
}
else {
while (--i > -1)
pthread_join(workers[i], NULL);
i=0;
}
x++;
}
while (--i > -1)
pthread_join(workers[i], NULL);
y++;
}
}
One trick I like to use to find issues like this (which I used in this case) is to change all fixed arrays to dynamically allocated memory. This does 2 things: it makes crashes more likely if you're doing something wrong (making it easier to find them), and it better allows tools like valgrind to detect exactly what's happening.
Upvotes: 3
Reputation: 33621
You only do ++x
on your if
but not the else
. So, the while
loop for x
may never complete.
I do not see a race condition that would allow the printf
to alter the timing and interaction of the threads, so I'm a bit mystified as to why you think it helps completion. When I initially ran your code (with the printf
), I still got an infinite loop.
Edit: After looking this over, the fix I came up with doesn't represent your loops correctly. It prevented an infinite loop, but did not cover the required x
range. There is a better fix below.
There is common code for your pthread_join
loops. By combining it and adding a break
, I [at least] got completion.
This may not be your total/final solution, but it may help:
#include <pthread.h>
#include <stdio.h>
#define WIDTH 1000
#define HEIGHT 1000
typedef struct s_pair {
int x;
int y;
} t_pair;
char pixels[HEIGHT][WIDTH];
void *
add_to_global(void *param)
{
t_pair *input = param;
if (! (input->x % 2))
pixels[input->y][input->x] = '.';
else if (! (input->x % 3))
pixels[input->y][input->x] = '#';
else
pixels[input->y][input->x] = ' ';
return (NULL);
}
void
thread_runner(int x_start, int x_end, int y_start, int y_end)
{
pthread_t workers[10];
t_pair inputs[10];
int i, x, y;
y = y_start - 1;
while (++y < y_end) {
x = x_start - 1;
i = -1;
while (x < x_end) {
if (++i >= 10)
break;
inputs[i] = (t_pair) { ++x, y };
pthread_create(&workers[i], NULL, add_to_global, &inputs[i]);
}
while (--i > -1)
pthread_join(workers[i], NULL);
// if this is removed, this program will never finish
printf("%s\n", pixels[y]);
}
}
int
main(void)
{
thread_runner(0, WIDTH, 0, HEIGHT);
return (0);
}
UPDATE:
Here is a better fix.
Your loops starting at -1
(e.g. y = y_start - 1
) are a bit uncommon and they complicate the code and may introduce off by one errors.
#include <pthread.h>
#include <stdio.h>
#define WIDTH 1000
#define HEIGHT 1000
typedef struct s_pair {
int x;
int y;
} t_pair;
char pixels[HEIGHT][WIDTH];
void *
add_to_global(void *param)
{
t_pair *input = param;
if (! (input->x % 2))
pixels[input->y][input->x] = '.';
else if (! (input->x % 3))
pixels[input->y][input->x] = '#';
else
pixels[input->y][input->x] = ' ';
return (NULL);
}
void
thread_join(int i,pthread_t *workers)
{
while (--i > -1)
pthread_join(workers[i], NULL);
}
void
thread_runner(int x_start, int x_end, int y_start, int y_end)
{
pthread_t workers[10];
t_pair inputs[10];
int i, x, y;
for (y = y_start; y < y_end; ++y) {
x = x_start;
while (x < x_end) {
for (i = 0; (i < 10) && (x < x_end); ++i, ++x) {
inputs[i] = (t_pair) { x, y };
pthread_create(&workers[i], NULL, add_to_global, &inputs[i]);
}
thread_join(i,workers);
i = 0;
}
// NOTE: not really needed -- just to be safe
thread_join(i,workers);
// if this is removed, this program will never finish
#if 0
printf("%s\n", pixels[y]);
#else
printf("y=%d x=%d x_end=%d\n",y,x,x_end);
#endif
}
}
int
main(void)
{
thread_runner(0, WIDTH, 0, HEIGHT);
return (0);
}
Upvotes: 0