pthreads + semaphores, why is this not executing properly?

Question

This is an assignment I'm working on. It must use semaphores, not mutex.

#include <stdio.h> 
#include <pthread.h> 
#include <assert.h> 
#include <unistd.h> 
#include <semaphore.h> 
#include <fcntl.h>
sem_t *ab, *ac, *ad, *de, *ce, *bf, *ef; 

void *a(void *arg) {
    printf("Entering A...\n");
    sleep(1);
    printf("Exiting A...\n");
    assert(sem_post(ab)==0);
    assert(sem_post(ac)==0);
    assert(sem_post(ad)==0);
    pthread_exit((void *)99);
}

void *b(void *arg) {
    assert(sem_wait(ab)==0);
    printf("Entering B...\n");
    sleep(1);
    printf("Exiting B...\n");
    assert(sem_post(bf)==0);
    pthread_exit((void *)99);
}

void *c(void *arg) {
    assert(sem_wait(ac)==0);
    printf("Entering C...\n");
    sleep(1);
    printf("Exiting C...\n");
    assert(sem_post(ce)==0);
    pthread_exit((void *)99);
}

void *d(void *arg) {
    assert(sem_wait(ad)==0);
    printf("Entering D...\n");
    sleep(1);
    printf("Exiting D...\n");
    assert(sem_post(de)==0);
    pthread_exit((void *)99);
}

void *e(void *arg) {
    assert(sem_wait(ce)==0);
    assert(sem_wait(de)==0);
    printf("Entering E...\n");
    sleep(1);
    printf("Exiting E...\n");
    assert(sem_post(ef)==0);
    pthread_exit((void *)99);
}

void *f(void *arg) {
    assert(sem_wait(bf)==0);
    assert(sem_wait(ef)==0);
    printf("Entering F...\n");
    sleep(1);
    printf("Exiting F...\n");
    pthread_exit((void *)99);
}


int main() { 
    pthread_t _a, _b, _c, _d, _e, _f;
    int r1, r2, r3, r4, r5, r6;

    ab=sem_open("foobar", O_CREAT, 0700, 0);
    ac=sem_open("foobar", O_CREAT, 0700, 0);
    ad=sem_open("foobar", O_CREAT, 0700, 0);
    ce=sem_open("foobar", O_CREAT, 0700, 0);
    de=sem_open("foobar", O_CREAT, 0700, 0);
    ef=sem_open("foobar", O_CREAT, 0700, 0);
    bf=sem_open("foobar", O_CREAT, 0700, 0);

    /*sem_init(ab,0,1);
    sem_init(ac,0,1);
    sem_init(ad,0,1);
    sem_init(ce,0,1);
    sem_init(de,0,1);
    sem_init(ef,0,1);
    sem_init(bf,0,1);*/

    assert(pthread_create(&_a, NULL, a, &r1) == 0);
    assert(pthread_create(&_b, NULL, b, &r2) == 0);
    assert(pthread_create(&_c, NULL, c, &r3) == 0);
    assert(pthread_create(&_d, NULL, d, &r4) == 0);
    assert(pthread_create(&_e, NULL, e, &r5) == 0);
    assert(pthread_create(&_f, NULL, f, &r6) == 0);

    assert(pthread_join(_a, NULL) == 0);
    assert(pthread_join(_b, NULL) == 0);
    assert(pthread_join(_c, NULL) == 0);    
    assert(pthread_join(_d, NULL) == 0);
    assert(pthread_join(_e, NULL) == 0);
    assert(pthread_join(_f, NULL) == 0);

    assert( sem_close(ab)==0 ); 
    assert( sem_close(ac)==0 ); 
    assert( sem_close(ad)==0 ); 
    assert( sem_close(ce)==0 );
    assert( sem_close(de)==0 ); 
    assert( sem_close(bf)==0 );
    assert( sem_close(ef)==0 ); 

    return 0; 
}

It's pretty simple but for some reason it's not executing in the right order. The output is far from consistent but always incorrect. Here is one sample output:

Entering A...
Entering B... <----sem_post(ab) has not even been called yet
Exiting A...
Entering C...
Entering D...
Exiting B...
Exiting D...
Exiting C...
Entering E...
Entering F...
Exiting F...
Exiting E...

It should be following this diagram:

Any help with this will be greatly appreciated, but it's an assignment so don't start telling me to do it a completely different way and don't give the answer straight up, just point me in the right direction.

Answer 1

You have two problems. The first is that you only have one semaphore, called foobar, which you are opening seven times. The second problem is that named semaphores are persistant - they stay around (and maintain the same value) until you call sem_unlink() on them. Since you never do this, it's likely that the semaphore foobar starts with a value greater than zero, from a previous run of your program.

You can correct these problems by using sem_unlink() to ensure that the semaphores don't exist before you create them, and using a different name for each semaphore.

Alternatively, you should really be using unnamed semaphores, which are created with sem_init() instead of sem_open(). To do this, you would change your declarations of ab, ac, ... to:

sem_t ab, ac, ad, de, ce, bf, ef;

You would then change all of the sem_post() and sem_wait() calls so that they use &ab, &ac, ...:

void *a(void *arg) {
    printf("Entering A...\n");
    sleep(1);
    printf("Exiting A...\n");
    assert(sem_post(&ab)==0);
    assert(sem_post(&ac)==0);
    assert(sem_post(&ad)==0);
    pthread_exit((void *)99);
}

You would replace the sem_open() calls with sem_init():

sem_init(&ab, 0, 0);
sem_init(&ac, 0, 0);
sem_init(&ad, 0, 0);
sem_init(&ce, 0, 0);
sem_init(&de, 0, 0);
sem_init(&ef, 0, 0);
sem_init(&bf, 0, 0);

And finally replace the sem_close() calls with sem_destroy():

assert( sem_destroy(&ab)==0 );
assert( sem_destroy(&ac)==0 );
assert( sem_destroy(&ad)==0 );
assert( sem_destroy(&ce)==0 );
assert( sem_destroy(&de)==0 );
assert( sem_destroy(&bf)==0 );
assert( sem_destroy(&ef)==0 );

---

When I make the changes above to your code, I get the following output, which I believe is what you are expecting:

Entering A...
Exiting A...
Entering B...
Entering C...
Entering D...
Exiting B...
Exiting C...
Exiting D...
Entering E...
Exiting E...
Entering F...
Exiting F...

Answer 2

In your code A is not locked before printing the exit line with ab.

This means that when it returns from the sleep it can do whatever it wants, because it's not dependent on the lock or anything else.

You should use the semaphores to stop the other thread/functions from continuing. sleep() just gives up the processor the amount of time you specify, after that they can continue.

Note:

You probably don't have to use sleep. the threads will give up the CPU if they can't attain the lock