1

关于下面的代码，我想知道的是：在方法调用周围使用 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
4

2 回答 2

3

由于您使用的是 boost，我会考虑将互斥锁与 a 结合使用boost::scoped_lock，这样当scoped_lock对象超出范围时，互斥锁会通过其析构函数调用“自动”解锁。然后，您无需担心与您的try和catch块交错解锁互斥锁，因为通过异常展开堆栈将通过scoped_lock对象释放互斥锁上的锁。

于 2012-09-25T21:56:26.497 回答
0

不，您使用 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); 
}
于 2012-09-25T21:56:48.280 回答