Confusing core dump while implementing semaphore

Question

According to : http://www.haiku-os.org/legacy-docs/benewsletter/Issue1-26.html#Engineering1-26

I have the following implemented in g++ 4.4.6 !!

#ifndef BENAPHORE_
#define BENAPHORE_

#include <string>
#include <memory>
#include <utility>
#include <semaphore.h>
#include <pthread.h>
#include <assert.h>

class benaphore
{
private:
    char fname[128] ;
    sem_t* benaphore_sem ;
    int benaphore_atom ;
    bool closed ;
public:
    benaphore()
    {
        benaphore_atom = 0 ;
    }
    ~benaphore()
    {
        if(!closed)
        {
            sem_close(benaphore_sem) ;
            sem_unlink(fname) ;
        }
    }
    void close_benaphore()
    {
        sem_close(benaphore_sem) ;
        sem_unlink(fname) ;
        closed = true ;
    }
    void open_benaphore(char* fname_)
    {
        closed = false ;
        strcpy(fname,fname_) ;
        benaphore_sem = sem_open(fname,O_CREAT,0644,0);
        if(benaphore_sem == SEM_FAILED)
        {
            printf("benaphore sem_open error \n") ;
        }
    }
    void acquire_benaphore()
    {
        int previous_value;
        previous_value = __sync_fetch_and_add(&benaphore_atom, 1);

        if (previous_value > 0)
            sem_wait(benaphore_sem);
    }

    void release_benaphore()
    {
        int previous_value;
        previous_value = __sync_fetch_and_add(&benaphore_atom, -1);

        if (previous_value > 1)
            sem_post(benaphore_sem);
    }
} ;

#endif

and the test source :

#include "benaphore.hpp"

benaphore b1 ;
pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
int glbCnt[6]={0,0,0,0,0,0} ;
void gexit(int gub)
{
    b1.close_benaphore() ;
    for(int idx=0;idx<6;idx++)
        printf("(%d)\n",glbCnt[idx]) ;
    sleep(1) ;
    exit(0) ;
}
void *testx(void *arg)
{
    long ilocal = (long)arg ;
    for(int idx=0;idx<3000000;idx++)
    {
        //pthread_mutex_lock(&mutex);
        b1.acquire_benaphore() ;
        ++glbCnt[ilocal]  ;
        b1.release_benaphore() ;
        //pthread_mutex_unlock(&mutex);
    }
}
int main()
{
    char* fname ;
    fname = (char*) malloc(80) ;
    strcpy(fname,"test_benaphore1") ;
    b1.open_benaphore(fname) ;
    signal(SIGINT, gexit);
    signal(SIGTERM, gexit);

    pthread_t id[6];
    int iCPU[6]={0,1,2,3,4,5} ;
    pthread_create(&id[0],NULL,testx,(void *)(long)iCPU[0] );
    pthread_create(&id[1],NULL,testx,(void *)(long)iCPU[1] );
    pthread_create(&id[2],NULL,testx,(void *)(long)iCPU[2] );
    pthread_create(&id[3],NULL,testx,(void *)(long)iCPU[3] );
    pthread_create(&id[4],NULL,testx,(void *)(long)iCPU[4] );
    pthread_create(&id[5],NULL,testx,(void *)(long)iCPU[5] );

    int i ;
    for(i=0;i<6;++i){
        pthread_join(id[i],NULL);
    }
    b1.close_benaphore() ;
    for(int idx=0;idx<6;idx++)
        printf("(%d)\n",glbCnt[idx]) ;
}

Compile :

g++ --std=c++0x test_benaphore.cpp -lpthread -o test_benaphore.exe

Each time I can get the correct answer I want if let every thread finish. But if I send an interrupt signal by key "control-C", then gexit() function is executed and mostly application gets "Segmentation fault (core dumped)", about interrupt application 10 times, will get 9 times core dump, one time no core dump with number less than 3,000,000 printed.

I have no idea why this happens, any comments, suggestions are appreciated.

Answer 1

There's not much that it's safe to do from an async signal handler, see bottom of this page for a list. Note that exit and printf aren't on it. You may be able to eliminate the segfault by calling _exit instead.

A better design maybe is to avoid doing any complex processing in the signal handler at all, and just set a flag...

namespace
{
    volatile sig_atomic_t time_to_quit;
}

void gexit(int gub)
{
    time_to_quit = 1;
}

.. which your threads examine from time to time

for(int idx=0; !time_to_quit && idx<3000000;idx++)

Now main does all the dangerous cleanup, same as on the success path.

Answer 2

Your close_benaphore function isn't thread safe, but you let any thread that catches a control-C call it. Perhaps you want to block SIGINT in all threads but the first one?