lkallio
lkallio

Reputation: 21

C - Pthreads threads go into infinite loop unless I print the output in between

#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

Answers (2)

dbush
dbush

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

Craig Estey
Craig Estey

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

Related Questions