Правильный способ использования мьютекса
Что я хотел бы знать о приведенном ниже коде, так это то, что использование блоков try catch для вызова методов является хорошей практикой. Какие глупости в коде ниже?
#ifndef TIMER_H
#define TIMER_H
#include <boost/bind/bind.hpp>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <boost/function.hpp>
#include <boost/thread/thread.hpp>
#include <boost/thread/recursive_mutex.hpp>
#include <boost/shared_ptr.hpp>
#include <boost/make_shared.hpp>
#include <stdint.h>
#include <iostream>
#include <vector>
template <uint32_t tMilliSeconds>
class Timer {
private:
static Timer *_instance;
uint32_t mMilliSeconds;
boost::mutex mListMutex;
boost::thread mTimerThread;
std::vector<boost::function<void()> > mHandlerList;
Timer();
Timer(const Timer &other);
Timer &operator=(const Timer &other);
void Run();
public:
~Timer();
static boost::shared_ptr<Timer<tMilliSeconds> > Instance();
void AddHandler(boost::function<void()> tmpBoostFunction);
};
template <uint32_t tMilliSeconds>
Timer<tMilliSeconds>::Timer() :
mListMutex() {
mMilliSeconds = tMilliSeconds;
mTimerThread = boost::thread(
boost::bind(&Timer<tMilliSeconds>::Run, this));
}
template <uint32_t tMilliSeconds>
Timer<tMilliSeconds>::~Timer() {
mListMutex.lock();
try {
mTimerThread.detach();
} catch (...) {
mListMutex.unlock();
}
mListMutex.unlock();
}
template <uint32_t tMilliSeconds>
boost::shared_ptr<Timer<tMilliSeconds> >
Timer<tMilliSeconds>::Instance() {
if (!_instance) {
_instance = new Timer<tMilliSeconds>();
}
return boost::shared_ptr<Timer<tMilliSeconds> >(_instance);
}
template <uint32_t tMilliSeconds>
void Timer<tMilliSeconds>::Run() {
while(true) {
boost::this_thread::sleep(
boost::posix_time::milliseconds(mMilliSeconds));
mListMutex.lock();
for (std::vector<boost::function<void()> >::iterator vect_it =
mHandlerList.begin(); vect_it != mHandlerList.end();
++vect_it) {
try {
(*vect_it)();
} catch (...) {
mListMutex.unlock();
}
}
mListMutex.unlock();
}
}
template <uint32_t tMilliSeconds>
void Timer<tMilliSeconds>::AddHandler(
boost::function<void()> tmpBoostFunction) {
mListMutex.lock();
try {
mHandlerList.push_back(tmpBoostFunction);
} catch (...) {
mListMutex.unlock();
}
mListMutex.unlock();
}
#endif // TIMER_H
Решение
Так как вы используете повышение, я бы хотел использовать мьютекс в сочетании с boost::scoped_lock, так что когда scoped_lock объект выходит из области видимости, мьютекс «автоматически» разблокируется с помощью вызова деструктора. Тогда вам не нужно будет беспокоиться о чередовании разблокировки мьютекса с вашим try а также catch блоки, так как разматывание стека с помощью исключения снимет блокировку с мьютекса через scoped_lock объект.
Другие решения
Нет, вы неправильно используете блоки catch. Если возникают исключения, вы вызываете больше разблокировок, чем должно быть. Если lock() успешно тогда звоните unlock() один раз и только один раз. Вы должны использовать блокировочную обертку, которая управляет разблокировкой для вас, например:
class mutex_lock
{
private:
boost::mutex &mListMutex;
public:
mutex_lock(boost::mutex &aListMutex) : mListMutex(aListMutex) { mListMutex.lock(); }
~mutex_lock() { mListMutex.unlock(); }
};
Тогда вы можете сделать это:
template <uint32_t tMilliSeconds>
Timer<tMilliSeconds>::~Timer()
{
mutex_lock lock(mListMutex);
mTimerThread.detach();
}
template <uint32_t tMilliSeconds>
void Timer<tMilliSeconds>::Run()
{
while(true) {
boost::this_thread::sleep(
boost::posix_time::milliseconds(mMilliSeconds));
mutex_lock lock(mListMutex);
for (std::vector<boost::function<void()> >::iterator vect_it = mHandlerList.begin(); vect_it != mHandlerList.end(); ++vect_it) {
(*vect_it)();
}
}
}
template <uint32_t tMilliSeconds>
void Timer<tMilliSeconds>::AddHandler(
boost::function<void()> tmpBoostFunction)
{
mutex_lock lock(mListMutex);
mHandlerList.push_back(tmpBoostFunction);
}
Обновить: Boost имеет свой собственный scoped_lock Класс для этой же цели:
#include <boost/interprocess/sync/scoped_lock.hpp>
template <uint32_t tMilliSeconds>
Timer<tMilliSeconds>::~Timer()
{
boost::interprocess::scoped_lock<boost::mutex> lock(mListMutex);
mTimerThread.detach();
}
template <uint32_t tMilliSeconds>
void Timer<tMilliSeconds>::Run()
{
while(true) {
boost::this_thread::sleep(
boost::posix_time::milliseconds(mMilliSeconds));
boost::interprocess::scoped_lock<boost::mutex> lock(mListMutex);
for (std::vector<boost::function<void()> >::iterator vect_it = mHandlerList.begin(); vect_it != mHandlerList.end(); ++vect_it) {
(*vect_it)();
}
}
}
template <uint32_t tMilliSeconds>
void Timer<tMilliSeconds>::AddHandler(
boost::function<void()> tmpBoostFunction)
{
boost::interprocess::scoped_lock<boost::mutex> lock(mListMutex);
mHandlerList.push_back(tmpBoostFunction);
}