patterncppMinor
Correct multithreaded reader-writer implementation
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
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_
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.
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.