Non-blocking solution to the dining philosophers

This is one of tasks, which I had to do for the recruitment process to be started in some company. Unfortunately they didn't like it so I've decided to share it here for discussion.

Philo5.h

#ifndef PHILO5_H_
#define PHILO5_H_

//Number of philosophers
const int PHIL_COUNT = 5;

//How many times each philosopher will eat
const int SEQ_COUNT = 5;

class CFork
{
    pthread_mutex_t _lock;

public:

    CFork();

    virtual ~CFork();

    void acquire();

    void release();

};

class CBoard
{
    pthread_mutex_t _lock;

public:
    explicit CBoard();

    virtual ~CBoard();

    void showChar(int no, char c);
};

class CPhilo
{
    int         _no;
    pthread_t   _thread;
    int         _tid;
    CFork*      _left;
    CFork*      _right;
    CBoard*     _board;

public:

    explicit CPhilo(int no, CFork* left, CFork* right, CBoard* board);

    virtual ~CPhilo();

    static void* run(void * context);

    void acquire();

    void release();

    void think();

    void eat();

    void waitForLeft();

    void waitForRight();

    void releaseLeft();

    void releaseRight();

    int getNo();

    void setNo(int no);

    bool amIOdd();

    void join();

};

//A class demonstrating problem of 5 philosophers
class CPhilo5
{
    CPhilo* philos[PHIL_COUNT];
    CFork * forks[PHIL_COUNT];
    CBoard* _board;

public:

    CPhilo5();

    ~CPhilo5();

    void join();

private:

    void createForks();

    void createPhilosophers();

};

#endif /* PHILO5_H_ */

Philo5.cpp

```
#include
#include //for sleep
#include //for rand
#include

#include "Philo5.h"

using namespace std;

CFork::CFork()
{
pthread_mutex_init(&_lock, NULL);
}

CFork::~CFork()
{
pthread_mutex_destroy(&_lock);
}

void CFork::acquire()
{
pthread_mutex_lock(&_lock);
}

void CFork::release()
{
pthread_mutex_unlock(&_lock);
}

//----------------------------------------------------------------------
CBoard::CBoard()
{
pthread_mutex_init(&_lock, NULL);
}

CBoard::~

Don't use all caps for language scope identifiers.

const int PHIL_COUNT = 5;

By convention identifiers in all caps are reserved for the pre-processor and have the potential to be stomped all over.

Stop using new

_board = new CBoard();

Why this can just as easily be an automatic variable. If I was reading this as part of your job application. I would just throw your resume away (not even file it for later consideration).

• Nearly all your variables can be automatic and thus not have any of the memory management issues.

• Also you are not using standard memory management techniques to control your dynamically allocated memory (thing std::shared_ptr and std::unqiue_ptr).

• There is not "Separation of Concerns" in your class. A class should either do business logic or it should do resource management it should never do both.

• You don't implement the rule of three/five.

Threading

Sure you can use the C language threading model. But C++ has its own threading model you should probably look into.

std::thread  thread;

Prefer reference over pointers

CFork*      _left;
CFork*      _right;
CBoard*     _board;

All these are guaranteed not to be NULL. So why have pointers. Reference are a much better metaphor for what you are trying to achieve and there is no need to validate they are NULL.

Public Interface

These are all part of the public interface?

void acquire();
void release();
void think();
void eat();
void waitForLeft();
void waitForRight();
void releaseLeft();
void releaseRight();
int getNo();
void setNo(int no);
bool amIOdd();
void join();

Overall Design

Is wrong. If a philosopher can not acquire either fork they are supposed to be thinking instead. In your code when they try and acquire a fork they block waiting for the to be available. Thus they never get any thinking time in.

When you try and acquire a fork if you fail you drop any currently held forks and then think. You should never block waiting for fork.

I don't remember and dinner parties were guests were just looking at the handle of the fork waiting for it to become available. If they could not get a fork they generally engage in conversation until the fork is available.

Remember this problem was originally designed to allocate tape drive resources as efficiently as possible. You are not using your tape drives or your CPU very well if you spend time waiting.

StackExchange Code Review Q#94943, answer score: 3