boost::try_mutex mutex--code review/your advise-comments

[g]

hello!

here is some code:


#ifndef RESOURCE_H_
#define RESOURCE_H_

#include <boost/date_time/gregorian/gregorian.hpp>
#include <boost/thread/mutex.hpp>
#include <map>



class Resource
{
public:
Resource(int id,bool free);
virtual ~Resource();
bool findAvailiable(const boost::gregorian::date_period&);
bool checkPeriod(const boost::gregorian::date_period&);
bool addReservation(const boost::gregorian::date_period&);
int id()const {return ID;}
void delOutOfDate();
bool trylock()
{
return try_lock.try_lock();
}
void unlock()
{
try_lock.unlock();
}
private:
int ID;
bool free_;
typedef std::map<boost::gregorian::date,boost::gregorian::date_period>
Reserved;
Reserved reserved;
Reserved::iterator iter;
Reserved::iterator range_start;
Reserved::iterator range_end;
boost::try_mutex mutex;
boost::try_mutex::scoped_try_lock try_lock;

};

#endif /*RESOURCE_H_*/





#include "Resource.h"

Resource::Resource(int id,bool
free):ID(id),free_(free),iter(reserved.begin()),try_lock(mutex,false)
{

}

Resource::~Resource()
{
try_lock.unlock();
}

bool Resource::findAvailiable(const boost::gregorian::date_period&
period)
{
boost::gregorian::days days_length=period.length();
boost::gregorian::days size(1);
int length=days_length.days();
if(length>15)
{
boost::gregorian::date_duration d(2);
size=d;
}
short a=0;
while(a<3)
{
period.begin()+size;
period.end()-size;
if(checkPeriod(period))return true;
a++;
}
return false;
}

bool Resource::checkPeriod(const boost::gregorian::date_period& period)
{
range_start=reserved.lower_bound(period.begin());
range_end=reserved.upper_bound(period.end());
while(range_start!=range_end)
{
if(period.intersects(range_start->second))return false;
range_start++;
}
return true;
}

bool Resource::addReservation(const boost::gregorian::date_period&
period)
{
range_start=reserved.lower_bound(period.begin());
range_end=reserved.upper_bound(period.end());
while(range_start!=range_end)
{
if(period.intersects(range_start->second))return false;
range_start++;
}
reserved.insert(std:air<boost::gregorian::date,boost::gregorian::date_period>(period.begin(),period));
return true;
}

void Resource::delOutOfDate()
{
boost::gregorian::date now(boost::gregorian::day_clock::local_day());
while(iter != reserved.end())
{
if(iter->second.last()<now)
{
reserved.erase(iter->second.begin());
}
}
}


the way I access resources


bool ResourceManager::reserveResource(const
boost::gregorian::date_period& period)
{
Reserved::iterator iter;
iter=reserved.begin();
while(iter!=reserved.end())
{
if(iter->second->trylock())
{
if(iter->second->addReservation(period))
{
iter->second->unlock();
return true;
}
iter->second->unlock();
}
iter++;
}
return false;
}


is this thread-safe enough??

I will use a singleton for the ResourceManager
Singleton<ResourceManager>::instance()->reserveResource(some_period);
having this design I can( I hope! ) have n parallel accesses(searching
/ reserve)
where n is the number of resources.
any sugestion in general?

thanks!

 

[Axter]

hello!

here is some code:


#ifndef RESOURCE_H_
#define RESOURCE_H_

#include <boost/date_time/gregorian/gregorian.hpp>
#include <boost/thread/mutex.hpp>
#include <map>



class Resource
{
public:
Resource(int id,bool free);
virtual ~Resource();
bool findAvailiable(const boost::gregorian::date_period&);
bool checkPeriod(const boost::gregorian::date_period&);
bool addReservation(const boost::gregorian::date_period&);
int id()const {return ID;}
void delOutOfDate();
bool trylock()
{
return try_lock.try_lock();
}
void unlock()
{
try_lock.unlock();
}
private:
int ID;
bool free_;
typedef std::map<boost::gregorian::date,boost::gregorian::date_period>
Reserved;
Reserved reserved;
Reserved::iterator iter;
Reserved::iterator range_start;
Reserved::iterator range_end;
boost::try_mutex mutex;
boost::try_mutex::scoped_try_lock try_lock;

};

#endif /*RESOURCE_H_*/





#include "Resource.h"

Resource::Resource(int id,bool
free):ID(id),free_(free),iter(reserved.begin()),try_lock(mutex,false)
{

}

Resource::~Resource()
{
try_lock.unlock();
}

bool Resource::findAvailiable(const boost::gregorian::date_period&
period)
{
boost::gregorian::days days_length=period.length();
boost::gregorian::days size(1);
int length=days_length.days();
if(length>15)
{
boost::gregorian::date_duration d(2);
size=d;
}
short a=0;
while(a<3)
{
period.begin()+size;
period.end()-size;
if(checkPeriod(period))return true;
a++;
}
return false;
}

bool Resource::checkPeriod(const boost::gregorian::date_period& period)
{
range_start=reserved.lower_bound(period.begin());
range_end=reserved.upper_bound(period.end());
while(range_start!=range_end)
{
if(period.intersects(range_start->second))return false;
range_start++;
}
return true;
}

bool Resource::addReservation(const boost::gregorian::date_period&
period)
{
range_start=reserved.lower_bound(period.begin());
range_end=reserved.upper_bound(period.end());
while(range_start!=range_end)
{
if(period.intersects(range_start->second))return false;
range_start++;
}
reserved.insert(std:air<boost::gregorian::date,boost::gregorian::date_period>(period.begin(),period));
return true;
}

void Resource::delOutOfDate()
{
boost::gregorian::date now(boost::gregorian::day_clock::local_day());
while(iter != reserved.end())
{
if(iter->second.last()<now)
{
reserved.erase(iter->second.begin());
}
}
}


the way I access resources


bool ResourceManager::reserveResource(const
boost::gregorian::date_period& period)
{
Reserved::iterator iter;
iter=reserved.begin();
while(iter!=reserved.end())
{
if(iter->second->trylock())
{
if(iter->second->addReservation(period))
{
iter->second->unlock();
return true;
}
iter->second->unlock();
}
iter++;
}
return false;
}


is this thread-safe enough??

I don't see any thing checking for a successful lock.
try_lock logic should be used with is_lock logic to verify the lock is
successful.
If it's not going to check, then you should use lock instead of try_lock

 

[g]

if(iter->second->trylock())

this is the check!
trylock() returns true/false.

 

[g]

bool trylock()//should be exception-safe
{
try{
try_lock.try_lock();
return true;
catch(boost::lock_error& e){}
return false;
}

I am still waiting for your advises