-2

Consider the following program,

static long count = 0;
void thread()
{
    printf("%d\n",++count);
}
int main()
{
    pthread_t t;
    sigset_t set;
    int i,limit = 30000;
    struct rlimit rlim;

    getrlimit(RLIMIT_NPROC, &rlim);
    rlim.rlim_cur = rlim.rlim_max;
    setrlimit(RLIMIT_NPROC, &rlim);

    for(i=0; i<limit; i++) {
        if(pthread_create(&t,NULL,(void *(*)(void*))thread, NULL) != 0) {
            printf("thread creation failed\n");
            return -1;
        }
    }
    sigemptyset(&set);
    sigsuspend(&set);
    return 0;
}

This program is expected to print 1 to 30000. But it some times prints 29945, 29999, 29959, etc. Why this is happening?

  • 2
    why would it be expected to print so? – Antti Haapala -- Слава Україні Aug 14 '19 at 12:45
  • 2
    Why is your thread function not using the proper signature? – Shawn Aug 14 '19 at 12:46
  • 3
    That unprotected use of `count` is a race condition; consider using C11 atomic types. – Shawn Aug 14 '19 at 12:48
  • 1
    Possible duplicate of [pthread execution on linux](https://stackoverflow.com/questions/4991470/pthread-execution-on-linux), [Pthread Run a thread right after it's creation](https://stackoverflow.com/q/12536649/608639), etc. – jww Aug 14 '19 at 12:49
  • 1
    Why shouldn't it print 29945, 29999, or 29959? Those are all between 1 and 30000. – Kevin Aug 14 '19 at 12:51
  • @Kevin - my guess is what OP means is that it doesn't always print all the way to 30000 as OP expected, but rather comes up short sometimes (due to the unprotected use of count). – pstrjds Aug 14 '19 at 12:53
  • 1
    You should also think about what happens when you return from `main()` before all those threads have finished. – Shawn Aug 14 '19 at 13:07

2 Answers2

1

Because count isn't atomic, so you have a race condition both in the increment and in the subsequent print.

The instruction you need is atomic_fetch_add, to increment the counter and avoid the race condition. The example on cppreference illustrates the exact problem you laid out.

Your example can be made to work with just a minor adjustment:

#include <stdio.h>
#include <signal.h>
#include <sys/resource.h>
#include <pthread.h>
#include <stdatomic.h>

static atomic_long count = 1;
void * thread(void *data)
{
    printf("%ld\n", atomic_fetch_add(&count, 1));
    return NULL;
}

int main()
{
    pthread_t t;
    sigset_t set;
    int i,limit = 30000;
    struct rlimit rlim;

    getrlimit(RLIMIT_NPROC, &rlim);
    rlim.rlim_cur = rlim.rlim_max;
    setrlimit(RLIMIT_NPROC, &rlim);

    for(i=0; i<limit; i++) {
        if(pthread_create(&t, NULL, thread, NULL) != 0) {
            printf("thread creation failed\n");
            return -1;
        }
    }
    sigemptyset(&set);
    sigsuspend(&set);
    return 0;
}

I made a handful of other changes, such as fixing the thread function signature and using the correct printf format for printing longs. But the atomic issue is why you weren't printing all the numbers you expected.

nickelpro
  • 2,537
  • 1
  • 19
  • 25
  • "The instruction you need is atomic_fetch_add" -- no, he needs a mutex. Atomic variables are *not* appropriate for beginners. – Employed Russian Aug 14 '19 at 19:49
  • @EmployedRussian No he doesn't? `atomic_fetch_add` will give him a copy of the data in the atomic and atomically increment the counter. Counters is explicitly the use case `atomic_fetch_add` is used for. Do you have an explanation why it wouldn't work? It's not like mutexs are easier to understand than atomic, mutexs _are_ atomics under the hood. – nickelpro Aug 14 '19 at 19:50
  • It would work for a trivial program like this one, but is not an appropriate solution for almost all real problems. You are saying: "here, use this grenade to open a bottle". A bottle opener is more appropriate tool. – Employed Russian Aug 14 '19 at 19:53
  • "Is not an appropriate solution for almost all real problems" _It's a counter_, atomic counters are everywhere in threaded programming. They're the sync primitive for a whole class of low-contention semaphores. Just going to have to agree to disagree here. – nickelpro Aug 14 '19 at 19:58
0

Why this is happening?

Because you have a data race (undefined behavior).

In particular, this statement:

printf("%d\n",++count);

modifies a global (shared) variable without any locking. Since the ++ does not atomically increment it, it's quite possible for multiple threads to read the same value (say 1234), increment it, and store the updated value in parallel, resulting in 1235 being printed repeatedly (two or more times), and one or more of the increments being lost.

A typical solution is to either use mutex to avoid the data race, or (rarely) an atomic variable (which guarantees atomic increment). Beware: atomic variables are quite hard to get right. You are not ready to use them yet.

Employed Russian
  • 199,314
  • 34
  • 295
  • 362