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

Correct multithreaded reader-writer implementation

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

Problem

I think I'm finished writing a multithreaded reader-writer implementation for my Operating Systems course. I would like to verify that my multithreading is correct and that I'm using good C++0x style.

PalindromeDatabase.h

class PalindromeDatabase : public Utility::Uncopyable
{
    friend void *read(void *classHandle);
    friend void *write(void *classHandle);

public:
    PalindromeDatabase(std::vector const& arguments);

    void process();

private:
    void createReaderThreads();
    void createWriterThreads();

    void joinReaderThreads();
    void joinWriterThreads();

    void *read();
    void readPalindromes();

    void *write();
    void processPalindromes();
    void formatPalindrome(std::string& palindrome);
    void removeNonAlphabeticCharacters(std::string& palindrome);
    void toLowerCase(std::string& palindrome);

private:
    const std::string PalindromeFilename_;

    const int ThreadWorkTime_;

    const int NumberOfReaders_;
    const int NumberOfWriters_;

    std::vector readerThreads_;
    std::vector writerThreads_;

    sem_t semaphore_;
    pthread_mutex_t writerMutex_;

    Timer timer_;
};


PalindromeDatabase.cpp

```
PalindromeDatabase::PalindromeDatabase(std::vector const& arguments)
: PalindromeFilename_("palindromes"),
ThreadWorkTime_(arguments[1]),
NumberOfReaders_(arguments[2]), NumberOfWriters_(arguments[3]),
readerThreads_(NumberOfReaders_), writerThreads_(NumberOfWriters_)
{
sem_init(&semaphore_, 0, NumberOfReaders_);
pthread_mutex_init(&writerMutex_, nullptr);
}

void PalindromeDatabase::process()
{
Logger().log() (this));
}

void PalindromeDatabase::joinReaderThreads()
{
for (auto& thread : readerThreads_)
{
int garbage = 0;
pthread_join(thread, reinterpret_cast(&garbage));
}
}

void *PalindromeDatabase::read()
{
timeval startTime;
gettimeofday(&startTime, nullptr);

while (Utility::elapsedTime(startTime) < ThreadWorkTime_)
{
sem_

Solution

It is very hard to determine if multi threaded code is correct just by looking at it. You should create special tests that simulate concurrent operations and validate shared state for corruption. The test(s) should be run for considerable amount of time and preferably on multi CPU (core) environment to be sure that code is working correctly.

Here's what I consider suspicious in the code sample you've provided:
In PalindromeDatabase::write() you're releasing write mutex soon after you've waited for all readers to complete. If I understand correctly, if there are multiple writers it is possible that they will call processPalindromes() simultaneously.

I propose placing pthread_mutex_unlock(&writerMutex_); after the processPalindromes is called thus write will be thread-safe.

Context

StackExchange Code Review Q#7014, answer score: 2

Revisions (0)

No revisions yet.