Is implementing semaphore or mutex necessary for simple counter?

Question

I tried implementing programs that calculates sort of integral. And in order to speed up the computation, one creates multiple processes and other uses multiple threads. In my program, each process adds a double value into shared memory and each thread adds a double value through the pointer.

Here's my question. The add operation obviously loads the value from memory, add a value to that, and stores the result to the memory. So it seems my code is quite prone to producer-consumer problem as many processes/threads access the same memory area. However, I couldn't find the case where somebody used semaphores or mutexes to implement a simple accumulator neither.

// creating processes
while (whatever)
{
    pid = fork();
    if (pid == 0)
    {
        res = integralproc(clist, m, tmpcnt, tmpleft, tmpright);
        *(createshm(shm_key)) += res;
        exit(1);
    }
}
// creating or retrieving shared memory
long double* createshm(int key)
{
    int shm_id = -1;
    void* shm_ptr = (void*)-1;
    while (shm_id == -1)
    {
        shm_id = shmget((key_t)key, sizeof(long double), IPC_CREAT | 0777);
    }
    while (shm_ptr == (void*)-1)
    {
        shm_ptr = shmat(shm_id, (void*)0, 0);
    }
    return (long double*)shm_ptr;
}

// creating threads
while (whatever)
{
    threadres = pthread_create(&(targs[i]->thread_handle), NULL, integral_thread, (void*)targs[i]);
}
// thread function. targ->resptr is pointer that we add the result to.
void *integral_thread(void *arg)
{
    threadarg *targ = (threadarg*)arg;
    long double res = integralproc(targ->clist, targ->m, targ->n, targ->left, targ->right);
    *(targ->resptr) += res;
    //printf("thread %ld calculated %Lf\n", targ->i, res);
    pthread_exit(NULL);
}

So I implemented it this way, and so far no matter how many processes/threads I make, the result was as if it never happened. I'm concerned that my codes may still be potentially dangerous, just barely out of my sight. Is this code truly safe from any of these problems? Or am I overlooking at something and should the code be revised?

Answer 1

If your threads are all racing to update the same object (ie, the targ->resptr for each thread points at the same thing), then yes - you do have a data race and you can see incorrect results (likely, "lost updates" where two threads that happen to finish at the same time try to update the sum, and only one of them is effective).

You probably haven't seen this because the execution time of your integralproc() function is long, so the chances of multiple threads simultaneously getting to the point of updating *targ->resptr is low.

Nonetheless, you should still fix the problem. You can either add a mutex lock/unlock around the sum update:

pthread_mutex_lock(&result_lock);
*(targ->resptr) += res;
pthread_mutex_unlock(&result_lock);

(This shouldn't affect the efficiency of the solution, since you are only locking and unlocking once in the lifetime of each thread).

Alternatively, you can have each thread record its own partial result in its own thread argument structure:

targ->result = res;

Then, once the worker threads have all been pthread_join()ed the parent thread that created them can just go through all the thread argument structures and add up the partial results.

No extra locking is needed here because the worker threads don't access each others result variable, and the pthread_join() provides the necessary synchronisation between the worker setting the result and the parent thread reading it.