Thread safe destruction of Read-Write Lock in C

Question

I'm trying to write a thread-safe read-write lock in C using POSIX semaphores. You can see the current state of the source code here. I followed this to create a readers-preferred lock.

The problem is I would like to handle the destruction of the lock for any possible state it may be when rwl_destroy() is called.

If destroy is called and no other thread is on the lock then it will lock the wrt (used by writers) to prevent any other thread from accessing the data guarded by the lock. Next the destroy function should destroy the semaphores and free the memory allocated for the ReadWriteLock struct. But what if another thread is now waiting on the lock? According to the documentation this thread will be left in an undefined state.

That's what I'm trying to avoid in order make lock easier to use.

EDIT:

the current code is:

typedef struct ReadWriteLock
{
sem_t wrt;
sem_t mtx;
sem_t delFlag;
int readcount;
int active;
}ReadWriteLock;

//forward declaration
/* This function is used to take the state of the lock.
 * Return values:
 *      [*] 1 is returned when the lock is alive.
 *      [*] 0 is returned when the lock is marked for delete.
 *      [*] -1 is returned if an error was encountered.
 */
int isActive(ReadWriteLock*);

int rwl_init(ReadWriteLock* lock)
{
lock = malloc(sizeof(ReadWriteLock));
if (lock == NULL)
{
    perror("rwl_init - could not allocate memory for lock\n");
    return -1;
}
if (sem_init(&(lock->wrt), 0, 1) == -1)
{
    perror("rwl_init - could not allocate wrt semaphore\n");
    free(lock);
    lock = NULL;
    return -1;
}
if (sem_init(&(lock->mtx), 0, 1) == -1)
{
    perror("rwl_init - could not allocate mtx semaphore\n");
    sem_destroy(&(lock->wrt));
    free(lock);
    lock = NULL;
    return -1;
}
if (sem_init(&(lock->delFlag), 0 , 1) == -1)
{
    perror("rwl_init - could not allocate delFlag semaphore\n");
    sem_destroy(&(lock->wrt));
    sem_destroy(&(lock->mtx));
    free(lock);
    lock = NULL;
    return -1;
}

lock->readcount = 0;
lock->active = 1;
return 0;
}

int rwl_destroy(ReadWriteLock* lock)
{
errno = 0;
if (sem_trywait(&(lock->wrt)) == -1)
    perror("rwl_destroy - trywait on wrt failed.");
if ( errno == EAGAIN)
    perror("rwl_destroy - wrt is locked, undefined behaviour.");

errno = 0;
if (sem_trywait(&(lock->mtx)) == -1)
    perror("rwl_destroy - trywait on mtx failed.");
if ( errno == EAGAIN)
    perror("rwl_destroy - mtx is locked, undefined behaviour.");

if (sem_destroy(&(lock->wrt)) == -1)
    perror("rwl_destroy - destroy wrt failed");
if (sem_destroy(&(lock->mtx)) == -1)
    perror("rwl_destroy - destroy mtx failed");
if (sem_destroy(&(lock->delFlag)) == -1)
    perror("rwl_destroy - destroy delFlag failed");

free(lock);
lock = NULL;
return 0;
}

int isActive(ReadWriteLock* lock)
{
errno = 0;
if (sem_trywait(&(lock->delFlag)) == -1)
{
    perror("isActive - trywait on delFlag failed.");
    return -1;
}
if ( errno == EAGAIN)
{//delFlag is down, lock is marked for delete
    perror("isActive - tried to lock but ReadWriteLock was marked for delete");
    return 0;
}
return 1;
}

I also have these functions:

int rwl_writeLock(ReadWriteLock*);

int rwl_writeUnlock(ReadWriteLock*);

int rwl_readLock(ReadWriteLock*);

int rwl_readUnlock(ReadWriteLock*);

So my question is how you change these functions in order to avoid the undefined state I described above. Is it even possible or do the user of this code should be responsible for releasing all locks before attempting to destroy the ReadWriteLock?

The isActive() function and the delFlag semaphore are not used currently, they were made in my attempt to solve the problem.

Answer 1

You should implement a "disposed" state of your ReadWriteLock instance (the "active" field looks appropriate, but you don't use it, why?).

Check it twice in rwl_writeLock / rwl_readLock, before and after the sem_wait() call. This trick is well-known as a "double-checking lock pattern". If you find your Lock to be deleted before entering sem_wait, just leave the function. If you find your Lock to be deleted after entering sem_wait, do sem_post immediately and leave.

In your destroy() routine, set active=0, then sem_post to both semaphores (don't bother if sem_post fails). If you still need the sem_destroy afterwards, usleep a bit (so all readers and writers have their time to receive the signal) and do sem_destroy.

P.S. You actually have no need to call sem_destroy if you are sure that the semaphore is not used anymore.