HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Accessing limited resources with Pthreads

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pthreadswithlimitedresourcesaccessing

Problem

I'm a Pthreads newbie and I've been giving myself a challenge: I want to have a resource that multiple threads can access at the same time as read. However, if a thread wants to write, then this need to be exclusive.

Now, I know that there are read/write locks designed specifically for this purpose. I would not do this in production code; this is just to learn. For simplicity, I assume there is only one thread that can write (but of course, multiple that read).

I have come up with two version. The first one does not use signals, the second one does. I tested it a little bit and both seem to work. However, multi-thread programming is tricky.

Could someone review the two versions and let me know whether there might be a bug that my tests missed?

Version 1

```
#include
#include
#include
#include
#include

class limited_resource
{
public:
limited_resource()
{
pthread_mutex_init(&d_mutext, NULL);
d_write_request = false;
d_nreading = 0;
}

virtual ~limited_resource()
{
pthread_mutex_destroy(&d_mutext);
}

void read(int thread_id)
{
// Do a stupid waiting
while(true)
{
pthread_mutex_lock(&d_mutext);
if(!d_write_request)
{
d_nreading++;
break;
}
pthread_mutex_unlock(&d_mutext);

sleep(1);
}
pthread_mutex_unlock(&d_mutext);

// Do the actual reading
char data[4];

for(int ii = 0; ii < 4; ++ii)
{
data[ii] = d_data[ii];
sleep(1); // Super slow reading to create bugs on purpose
}

printf("Thread %d read \"%c%c%c%c\"\n", thread_id, data[0], data[1], data[2], data[3]);

// Decrement the number of readers
pthread_mutex_lock(&d_mutext);
d_nreading--;

Solution

You forgot to remove the "stupid waiting" from version 2. Otherwise, it looks good. Not particularly efficient, but correct.

You can and should remove this chunk of code from version 2. The code before it already does this:

// Do a stupid waiting
    while(true)
        {
        pthread_mutex_lock(&d_mutext);
        if(d_nreading == 0)
            break;
        pthread_mutex_unlock(&d_mutext);

        sleep(1);
        }
    pthread_mutex_unlock(&d_mutext);


Here's an optimization for you. Change:

if(d_nreading == 0)
        pthread_cond_signal(&d_nreading_zero);


to

if( (d_nreading == 0) && d_write_request)
        pthread_cond_signal(&d_nreading_zero);


Reader/writer locks are based on the assumption that reading is frequent and writing is rare. So the common case would be that there's no writing. Your code makes an extra call into the pthread library for each read unlock, even that call is not needed.

Code Snippets

// Do a stupid waiting
    while(true)
        {
        pthread_mutex_lock(&d_mutext);
        if(d_nreading == 0)
            break;
        pthread_mutex_unlock(&d_mutext);

        sleep(1);
        }
    pthread_mutex_unlock(&d_mutext);
if(d_nreading == 0)
        pthread_cond_signal(&d_nreading_zero);
if( (d_nreading == 0) && d_write_request)
        pthread_cond_signal(&d_nreading_zero);

Context

StackExchange Code Review Q#15136, answer score: 3

Revisions (0)

No revisions yet.