Restricting resource access by type for many-to-many

Question

DISCLAIMER: this post contains edits done on the answers below, all the credits go to their respective owners.

I am trying to implement a problem which states that a resource may be used by two types of threads. There can be more threads of each type. (4 threads of type White and 6 of type Black). Any number of blacks can use the resource at the same time. Same goes for the whites one. I still cannot wrap my head around this...

I tried to implement this using mutexes. I also wanted to take into account the possible starvation this implementation might have, thus I decided to check if the number of served threads of a certain type has been reached, allowing the other type to do work. I cannot seem to be able to implement the latest.

I also wanted to take into account that whenever the other type wants to use the resource, it must await its turn, as well as the others to finish using the resource.

EDIT: I tried to use the solution of @Nominal-Animal, but it seems this deadlocks sometimes as well. Moreover, I added the missing turn to the struct. Now, I have got some extra question:

• this seems right, but doesn't work, why?
• why does need the double negation for the isBLack parameter inside bwlock_lock()

Now, for some code:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <wait.h>
#include <pthread.h>

#define WHITES 31
#define BLACKS 33
#define TYPES 2
#define W_ID 0
#define B_ID 1

struct bwlock
{
    pthread_mutex_t lock;    /* Protects the rest of the fields */
    pthread_cond_t wait[2];  /* To wait for their turn */
    volatile int waiting[2]; /* Number of threads waiting */
    volatile int running[2]; /* Number of threads running */
    volatile int started[2]; /* Number of threads started in this turn */
    const int limit[2];      /* Maximum number of starts per turn */
    volatile int black;      /* Black threads' turn */
    volatile int turn;       /*The turn */
};

#define BWLOCK_INIT(whites, blacks, turn)                         \
    {                                                             \
        PTHREAD_MUTEX_INITIALIZER,                                \
            {PTHREAD_COND_INITIALIZER, PTHREAD_COND_INITIALIZER}, \
            {0, 0}, {0, 0}, {0, 0}, {whites, blacks}, 0, turn     \
    }

struct bwlock resource = BWLOCK_INIT(4, 5, W_ID);

void bwlock_unlock(struct bwlock *bwl, const int isblack)
{
    const int black = !!isblack; /* 0 if white, 1 if black */

    pthread_mutex_lock(&(bwl->lock));

    /* This thread is no longer using the resource. */
    bwl->running[black]--;

    /* Was this the last of this color, with others waiting? */
    if (bwl->running[black] <= 0 && bwl->waiting[!black])
    {
        /* Yes. It's their turn. */
        if (bwl->turn == black)
        {
            bwl->turn = !black;
            /* Clear their started counter. */
            bwl->started[!black] = 0;
        }
        /* Wake them all up. */
        pthread_cond_broadcast(&(bwl->wait[!black]));
    }

    pthread_mutex_unlock(&(bwl->lock));
}

void bwlock_lock(struct bwlock *bwl, const int isblack)
{
    const int black = !!isblack; /* 0 if white, 1 if black */

    pthread_mutex_lock(&(bwl->lock));
    while (1)
    {

        /* No runners or waiters of the other color? */
        if (!(bwl->waiting[!black] < 1) && bwl->running[!black] < 1)
        {
            /* No; we can run. Does this change the turn? */
            if (bwl->turn != black)
            {
                bwl->turn = black;
                /* Clear started counter. */
                bwl->started[black] = 0;
            }
            break;
        }

        /* Still our turn, and not too many started threads? */
        if (bwl->turn == black && bwl->started[black] < bwl->limit[black])
            break;

        /* We must wait. */
        bwl->waiting[black]++;
        pthread_cond_wait(&(bwl->wait[black]), &(bwl->lock));
        bwl->waiting[black]--;
    }

    bwl->started[black]++;
    bwl->running[black]++;

    pthread_mutex_unlock(&(bwl->lock));
}

typedef struct
{
    int thread_id;
    char *type;
    int type_id;
} data;

void use_resource(int thread_id, char *type)
{
    printf("U: Thread %d of type %s is using the resource!\n", thread_id, type);
}

void take_resource(int thread_id, char *type, int type_id)
{
    printf("W:Thread %d of type %s is trying to get the resource!\n", thread_id, type);
    bwlock_lock(&resource, type_id);
    printf("W:Thread %d of type %sB got resource!\n", thread_id, type);
}

void release_resource(int thread_id, char *type, int type_id)
{
    bwlock_unlock(&resource, type_id);
    printf("R:Thread %d of type %s has released the resource!\n", thread_id, type);
}

void *doWork(void *arg)
{
    data thread_data = *((data *)arg);

    int thread_id = thread_data.thread_id;
    char *type = thread_data.type;
    int type_id = thread_data.type_id;
    take_resource(thread_id, type, type_id);
    use_resource(thread_id, type);
    release_resource(thread_id, type, type_id);

    return NULL;
}

data *initialize(pthread_t threads[], int size, char *type, int type_id)
{
    data *args = malloc(sizeof(data) * size);
    for (int i = 0; i < size; i++)
    {
        args[i].type = type;
        args[i].thread_id = i;
        args[i].type_id = type_id;
        pthread_create(&threads[i], NULL, doWork, (void **)&args[i]);
    }
    return args;
}

void join(pthread_t threads[], int size)
{
    for (int i = 0; i < size; i++)
    {
        pthread_join(threads[i], NULL);
    }
}

int main()
{
    pthread_t whites[WHITES];
    pthread_t blacks[BLACKS];
    char *white = "WHITE";
    char *black = "BLACK";
    data *w_args = initialize(whites, WHITES, white, W_ID);
    data *b_args = initialize(blacks, BLACKS, black, B_ID);

    join(whites, WHITES);
    join(blacks, BLACKS);

    free(w_args);
    free(b_args);

    return 0;
}

This has been compiled using gcc -g -o ba blacks_whites.c -Wall -Wextra -pthread .

Answer 1

Consider the following an extended comment to John Bollingers answer.

The rules OP described are incomplete. For example, consider the case where you have three black threads with the resource, one white thread waiting for the resource, and another black thread arrives desiring to grab the resource. What should happen? If the black thread always gets the resource, then it is possible for black (or white) threads to starve the other type of thread. If the ownership is changed to the other type immediately when possible, we lose most of the benefit of the concurrency across threads of the same type; possibly running just one thread of one type at a time, with all threads sequential, if the distribution of incoming thread types is roughly even!

There are several possible solutions. The one that seems to fit OP's problem statement is to allow up to N_black black threads to run with the resource, before switching to whites' turn if any are waiting; and up to N_white white threads to run with the resource, before switching to blacks' turn. (A time limit, a grace period when additional threads of the same type may also grab the resource, is probably what you'd actually use in practice.)

We can use the following structure to describe this kind of a lock:

struct bwlock {
    pthread_mutex_t    lock;        /* Protects the rest of the fields */
    pthread_cond_t     wait[2];     /* To wait for their turn */
    volatile int       waiting[2];  /* Number of threads waiting */
    volatile int       running[2];  /* Number of threads running */
    volatile int       started[2];  /* Number of threads started in this turn */
    const int          limit[2];    /* Maximum number of starts per turn */
    volatile int       black;       /* Black threads' turn */
};
#define BWLOCK_INIT(whites, blacks) \
    { PTHREAD_MUTEX_INITIALIZER, \
      { PTHREAD_COND_INITIALIZER, PTHREAD_COND_INITIALIZER }, \
      { 0, 0 }, { 0, 0 }, { 0, 0 }, { whites, blacks }, 0 \
    }

The lock mutex is only held while examining the fields, not during resource access.

(Also note that although black is initially 0, when there are no runners and no waiters, the turn will change, so it does not matter. The code will work exactly the same if the initial black is 1.)

Let's look at releasing the bwlock first, because it is the more interesting part; it is what controls the turn changes. Let's assume both lock and release have a isblack parameter (which is either 0 or 1). If the releasing thread is the last of its color, and threads of the other color are waiting, it will change the turn, and broadcast on the other color wait condition variable to wake them up:

void bwlock_unlock(struct bwlock *bwl, const int isblack)
{
    const int black = !!isblack;  /* 0 if white, 1 if black */

    pthread_mutex_lock(&(bwl->lock));

    /* This thread is no longer using the resource. */
    bwl->running[black]--;

    /* Was this the last of this color, with others waiting? */
    if ((bwl->running[black] <= 0) && (bwl->waiting[!black] > 0)) {
        /* Yes. It's their turn. */
        if (bwl->black == black) {
            bwl->black = !black;
            /* Clear their started counter. */
            bwl->started[!black] = 0;
        }
        /* Wake them all up. */
        pthread_cond_broadcast(&(bwl->wait[!black]));
    }

    pthread_mutex_unlock(&(bwl->lock));
    return;
}

Grabbing the bwlock is more complicated. The limit only applies if there are no threads of the other type waiting (because threads of a single color would deadlock without the other color if we did).

void bwlock_lock(struct bwlock *bwl, const int isblack)
{
    const int  black = !!isblack; /* 0 if white, 1 if black */

    pthread_mutex_lock(&(bwl->lock));
    while (1) {

        /* No runners or waiters of the other color? */
        if ((bwl->waiting[!black] < 1) && (bwl->running[!black] < 1)) {
            /* No; we can run. Does this change the turn? */
            if (bwl->black != black) {
                bwl->black = black;
                /* Clear started counter. */
                bwl->started[black] = 0;
            }
            break;
        }

        /* Still our turn, and not too many started threads? */
        if ((bwl->black == black) && (bwl->started[black] < bwl->limit[black]))
            break;

        /* We must wait. */
        bwl->waiting[black]++;
        pthread_cond_wait(&(bwl->wait[black]), &(bwl->lock));
        bwl->waiting[black]--;
    }

    bwl->started[black]++;
    bwl->running[black]++;

    pthread_mutex_unlock(&(bwl->lock));
}

The pthread_cond_wait(&(bwl->wait[black]), &(bwl->lock)) releases the lock and waits for a signal or broadcast on the condition variable. When signaled, it will re-acquire the lock before returning. (There is no race window on releasing the lock and waiting on the condition variable; it happens atomically, effectively at the same point in time.)

If you consider the logic above, the bwlock_unlock() handles the case when the last running thread of a specific color should handle the "baton" to the other thread set. The bwlock_lock() determines whether a thread can run with the resource, or needs to wait. It will only change the turn when there are no threads of the other color running or waiting their turn.

It is rather simple scheme, but you may need to think of several cases to understand its behaviour.

The started counter is cleared when the turn changes, and incremented for each thread that was started during that turn. When it reaches limit, no more threads of that type will start; they will wait for their turn.

Let's say limit is {3, 3}, and you have four threads of each type, and they basically all rush to the bwlock at the same time. Let's say the first thread to grab the lock is black. The three first black threads will run with the resource, and one black and four white threads will wait on the condition variable. When the turn changes, three of the white threads get to run; one white, a random one, will re-wait until next white turn.

Also, as Craig Estey points out in a comment to John Bollingers answer, this does not guarantee fairness among threads of the same type. For example, if A and B are of the same type, A accesses the bwlock-protected resource multiple times during their turn, and B just once, it is possible for B to have to wait indefinitely before getting a turn because A "hogs" all its slots.

To guarantee fairness, we'd need some sort of a ticket lock or ordered wait queues, so that we can wake up the limit longest-waiting threads of a specific color, instead of random waiting ones.

Answer 2

General comments

1. sleep() is pretty much never the right solution to synchronization problems.
2. Mutexes provide only mutual exclusion. If you need more inter-thread coordination than that then you should either choose a different kind of synchronization object (sometimes a semaphore is a better fit), or add another kind (condition variables are the usual companion to mutexes).
3. It's questionable style to split mutex lock and unlock into different functions. In the even that you must do so, at least prefer to set up a balanced pairs of functions that perform the locking and unlocking.

Some specifics

You have too many mutexes, and it's getting you into trouble. I see no need to maintain separate mutexes for the different kinds of resource-management metadata you maintain. At most, you need two mutexes: one for protecting the group-access metadata (turn, pending, current, and served), and maybe one for protecting the resource itself. It is not necessary for any thread to hold both of these at the same time, but you must ensure that no shared data is accessed without protection of the appropriate mutex.

You should use a condition variable to help with mediating access to the group-access metadata. Instead of sleeping, threads will wait on the CV when it is not their group's turn. This will integrate naturally with the mutex that protects the group-access metadata.

Although implementing these changes will require somewhat of a redesign, the result will be conceptually simpler and practically more robust. For example, the issue in take_resource() revolving around uncertainty over which mutexes have been acquired will go away, because only one mutex will be involved in that part in the first place.