CODEWITHSUNDEEP
cpthreads
ant2009
ant2009

Reputation: 22486

Using a global variable to control a while loop in a separate thread

gcc (GCC) 4.6.3 c89 valgrind-3.6.1

Hello,

Updated code snippet +++++++++++++++++++++++++++++++++++

void *thread_recv_fd()
{
    pthread_mutex_lock(&mutex_queue);
    while(start_receiving) {
        pthread_mutex_unlock(&mutex_queue);

        pthread_mutex_lock(&mutex_queue);
        queue_remove();
        pthread_mutex_unlock(&mutex_queue);

        usleep(500);
    }

    pthread_exit(NULL);
}

using the above locks looks very ugly now. And I am still getting a race error on the reading of the start_receiving. I have also declared it as volatile as well. +++++++++++++++++++++++++++++++++++++

I am running a while loop in a worker thread that polls every 1/2 second. I control it with a global variable start_receiving after the user enters ctrl-c it will change the value to false.

Having a global like this is it good practice?

The reason I asked as I get these 2 errors when I run helgrind:

==6814== Possible data race during write of size 4 at 0x601958 by thread #1
==6814==    at 0x401131: main (driver.c:78) 
==6814==  This conflicts with a previous read of size 4 by thread #2
==6814==    at 0x4012A7: thread_recv_fd (driver.c:127)

This are the lines of code:

driver.c:78 is this line of code start_receiving = FALSE;
driver.c:127 is this line of code while(start_receiving) in the while loop

Source code, just the important snippets:

static int start_receiving = FALSE;

int main(void)
{
    pthread_t thread_recv_id;
    pthread_attr_t thread_attr;

    /* Start polling as soon as thread has been created */
    start_receiving = TRUE;

    do {
        /* Start thread that will send a message */
        if(pthread_create(&thread_recv_id, &thread_attr, thread_recv_fd, NULL) == -1) {
            fprintf(stderr, "Failed to create thread, reason [ %s ]",
                    strerror(errno));
            break;
        }
    }
    while(0);

    /* Wait for ctrl-c */
    pause(); 

    /* Stop polling - exit while loop */
    start_receiving = FALSE;

    /* Clean up threading properties */
    pthread_join(thread_recv_id, NULL);

    pthread_exit(NULL);
}

void *thread_recv_fd()
{
    while(start_receiving) {
        pthread_mutex_lock(&mutex_queue);
        queue_remove();
        pthread_mutex_unlock(&mutex_queue);

        usleep(500);
    }

    pthread_exit(NULL);
}

Many thanks for any suggestions,

Upvotes: 0

Views: 2331

Answers (3)

caf
caf

Reputation: 239011

You could use atomics, but the simplest solution here is just to use locking around the controlling variable. For example:

static int start_receiving = FALSE;
static pthread_mutex_t start_receiving_lock = PTHREAD_MUTEX_INITIALIZER;

int is_receiving(void)
{
    int r;

    pthread_mutex_lock(&start_receiving_lock);
    r = start_receiving;
    pthread_mutex_unlock(&start_receiving_lock);

    return r;
}

Then in the thread function:

void *thread_recv_fd()
{
    while(is_receiving()) {
        pthread_mutex_lock(&mutex_queue);
        queue_remove();
        pthread_mutex_unlock(&mutex_queue);

        usleep(500);
    }

    pthread_exit(NULL);
}

..and in the main function:

pthread_mutex_lock(&start_receiving_lock);
start_receiving = 1;
pthread_mutex_unlock(&start_receiving_lock);

Upvotes: 1

dalazx
dalazx

Reputation: 149

Polling isn't a right way. I suggest you to read about conditional variables

Upvotes: 1

unwind
unwind

Reputation: 399793

No, it's very bad practice.

At the very least, the variable should be volatile to avoid being optimized out.

You should really look into using some real multi-thread primitives for this though, such as mutexes (to protect the shared state, so that the two threads aren't accessing it at the same time) and atomic variables (to make sure the state is thread-proof).

Upvotes: 3

Related Questions