PThread beginner - starting, syncing, stopping a worker thread

Question

I have the following manager<->worker situation:

class Manager {
private:
    pthread_attr_t workerSettings;
    pthread_t worker;
    pthread_cond_t condition;
    pthread_mutex_t mutex;
    bool workerRunning;

    static void* worker_function(void* args) {
        Manager* manager = (Manager*)args;

        while(true) {
            while(true) {
                pthread_mutex_lock(&manager->mutex);
                if(/* new data available */)
                {
                    /* copy new data from shared to thread memory */
                    pthread_mutex_unlock(&manager->mutex);
                }
                else
                {
                    pthread_mutex_unlock(&manager->mutex);
                    break;
                }

                /* process the data in thread memory */

                pthread_mutex_lock(&manager->mutex);
                /* copy results back to shared memory */
                pthread_mutex_unlock(&manager->mutex);
            }

            pthread_mutex_lock(&manager->mutex);

            // wait for new data to arrive
            while(manager->workerRunning && !/* new data available*/)
                pthread_cond_wait(&manager->condition, &manager->mutex);

            // check if we should continue running
            if(!manager->workerRunning)
            {
                pthread_mutex_unlock(&manager->mutex);
                break;
            }

            pthread_mutex_unlock(&manager->mutex);
        }

        pthread_exit(NULL);
        return NULL; // just to avoid the missing return statement compiler warning
    }

public:
    Manager() : workerRunning(true) {
        pthread_cond_init(&condition, NULL);
        pthread_mutex_init(&mutex, NULL);
        pthread_attr_init(&workerSettings);
        pthread_attr_setdetachstate(&workerSettings, PTHREAD_CREATE_JOINABLE);
        pthread_create(&worker, &workerSettings, worker_function, (void*)this);
    }

    // this *may* be called repeatedly or very seldom
    void addData(void) {
        pthread_mutex_lock(&mutex);
        /* copy new data into shared memory */
        pthread_cond_signal(&condition);
        pthread_mutex_unlock(&mutex);
    }

    ~Manager()
    {
        // set workerRunning to false and signal the worker
        pthread_mutex_lock(&mutex);
        workerRunning = false;
        pthread_cond_signal(&condition);
        pthread_mutex_unlock(&mutex);

        // wait for the worker to exit
        pthread_join(worker, NULL);

        // cleanup
        pthread_attr_destroy(&workerSettings);
        pthread_mutex_destroy(&mutex);
        pthread_cond_destroy(&condition);
    }
};

I'm not completely sure about this at several places:

• Is the fact that Manager spawns a new thread in its constructor considered a bad practice? (I will only have one Manager object, so i guess that should be fine)
• What about the pthread_exit - i see this in many tutorials but i don't quite get why it should be there? Can't i simply return the function to exit the thread? I also think the return NULL is dead code, but gcc warns when it's missing because it obviously can't know that pthread_exit already killed the thread at that point.
• About the constructor - can i destroy the thread attr object (workerSettings) immediately after spawning the thread or does it have to stay valid for the entire lifetime of the thread?
• About the destructor: Is this the right way to do this?

And most importantly:

• Do your experienced eyes see any synchronization issues there?

Thanks for your help!

Answer 1

You ask...

Is the fact that Manager spawns a new thread in its constructor considered a bad practice?

In most cases, RAII is good enough to approach the object creation and resource acquisition. In some cases you may want to achieve the deferred resource initialization: when you first construct an object and later you proceed with the initialization. This can be achieved, for example, via a ctor (either default or parameterized) and open/start routines. Though you may also do it in the ctor and achieve the deffered object creation by allocating the object in the process heap (via operator new). It depends on your requirements, software design considerations and corporate software development standards. So, you may create a thread in ctor, or may want or need to spawn it in the later stage of the application/object lifecycle.

What about the pthread_exit

It is not required. It terminates the calling thread, making its exit status available to any waiting threads (i.e. via pthread_join()). An implicit call to pthread_exit() occurs when any thread returns from its start routine. Basically, the pthread_exit() function provides an interface similar to exit() but on a per-thread basis (including cancelation cleanup handlers). But beware of calling pthread_exit() from cancelation cleanup handlers or from destructors of objects allocated in the TSD (thread-specific data area) - it can lead to undesirable side effects.

About the constructor - can i destroy the thread attr object (workerSettings) immediately after spawning the thread or does it have to stay valid for the entire lifetime of the thread?

Yes, you can destroy it right away: it will not affect already created threads.

About the destructor: Is this the right way to do this?

Same thing as for ctor: you may use dtor and close/stop routine or can do it all in the dtor: depends on your specific needs (e.g. object reusability etc). Just make the dtor not throw.

Do your experienced eyes see any synchronization issues there?

I may suggest using pthread_testcancel(), to introduce the explicit cancelation point in a thread, and issue pthread_cancel() + pthread_join() (should return PTHREAD_CANCELED) in the control thread to stop the child thread, instead of synch variable workerRunning. Of course, if it is applicable in your case.