Browse Source

[trac957] Fixed the bug that destructIntervalTimer segfaults.

  - pass boost::shared_ptr of IntervalTimerImpl wrapped with boost::bind
    as a handler of asio::deadline_timer
  - use boost::shared_ptr to hold pImpl
Yoshitaka Aharen 14 years ago
parent
commit
e47b0a39b7
2 changed files with 28 additions and 23 deletions
  1. 24 17
      src/lib/asiolink/interval_timer.cc
  2. 4 6
      src/lib/asiolink/interval_timer.h

+ 24 - 17
src/lib/asiolink/interval_timer.cc

@@ -19,6 +19,8 @@
 #include <netinet/in.h>
 
 #include <boost/bind.hpp>
+#include <boost/enable_shared_from_this.hpp>
+#include <boost/shared_ptr.hpp>
 
 #include <exceptions/exceptions.h>
 
@@ -29,7 +31,11 @@
 namespace isc {
 namespace asiolink {
 
-class IntervalTimerImpl {
+/// This class uses shared_from_this in its methods. It must live inside
+/// a shared_ptr.
+class IntervalTimerImpl :
+    public boost::enable_shared_from_this<IntervalTimerImpl>
+{
 private:
     // prohibit copy
     IntervalTimerImpl(const IntervalTimerImpl& source);
@@ -85,43 +91,44 @@ IntervalTimerImpl::setup(const IntervalTimer::Callback& cbfunc,
     // At this point the timer is not running yet and will not expire.
     // After calling IOService::run(), the timer will expire.
     update();
-    return;
 }
 
 void
 IntervalTimerImpl::update() {
-    if (interval_ == 0) {
-        // timer has been canceled.  Do nothing.
-        return;
-    }
     try {
         // Update expire time to (current time + interval_).
         timer_.expires_from_now(boost::posix_time::millisec(interval_));
+        // Reset timer.
+        timer_.async_wait(boost::bind(&IntervalTimerImpl::callback,
+                                      shared_from_this(),
+                                      asio::placeholders::error));
     } catch (const asio::system_error& e) {
         isc_throw(isc::Unexpected, "Failed to update timer");
+    } catch (const boost::bad_weak_ptr& e) {
+        isc_throw(isc::Unexpected, "Failed to update timer");
     }
-    // Reset timer.
-    timer_.async_wait(boost::bind(&IntervalTimerImpl::callback, this, _1));
 }
 
 void
-IntervalTimerImpl::callback(const asio::error_code& cancelled) {
+IntervalTimerImpl::callback(const asio::error_code& ec) {
     assert(interval_ != INVALIDATED_INTERVAL);
-    // Do not call cbfunc_ in case the timer was cancelled.
-    // The timer will be canelled in the destructor of asio::deadline_timer.
-    if (!cancelled) {
-        cbfunc_();
+    if (interval_ == 0 || ec) {
+        // timer has been canceled. Do nothing.
+    } else {
         // Set next expire time.
         update();
+        // Invoke the call back function.
+        cbfunc_();
     }
 }
 
-IntervalTimer::IntervalTimer(IOService& io_service) {
-    impl_ = new IntervalTimerImpl(io_service);
-}
+IntervalTimer::IntervalTimer(IOService& io_service) :
+    impl_(new IntervalTimerImpl(io_service))
+{}
 
 IntervalTimer::~IntervalTimer() {
-    delete impl_;
+    // Cancel the timer to make sure cbfunc_() will not be called any more.
+    cancel();
 }
 
 void

+ 4 - 6
src/lib/asiolink/interval_timer.h

@@ -16,6 +16,7 @@
 #define __ASIOLINK_INTERVAL_TIMER_H 1
 
 #include <boost/function.hpp>
+#include <boost/shared_ptr.hpp>
 
 #include <asiolink/io_service.h>
 
@@ -42,9 +43,6 @@ class IntervalTimerImpl;
 /// The call back function will not be called if the instance of this class is
 /// destroyed before the timer is expired.
 ///
-/// Note: Destruction of an instance of this class while call back is pending
-/// causes throwing an exception from \c IOService.
-///
 /// Sample code:
 /// \code
 ///  void function_to_call_back() {
@@ -100,12 +98,12 @@ public:
     /// \param interval Interval in milliseconds (greater than 0)
     ///
     /// Note: IntervalTimer will not pass \c asio::error_code to
-    /// call back function. In case the timer is cancelled, the function
+    /// call back function. In case the timer is canceled, the function
     /// will not be called.
     ///
     /// \throw isc::InvalidParameter cbfunc is empty
     /// \throw isc::BadValue interval is less than or equal to 0
-    /// \throw isc::Unexpected ASIO library error
+    /// \throw isc::Unexpected internal runtime error
     void setup(const Callback& cbfunc, const long interval);
 
     /// Cancel the timer.
@@ -127,7 +125,7 @@ public:
     long getInterval() const;
 
 private:
-    IntervalTimerImpl* impl_;
+    boost::shared_ptr<IntervalTimerImpl> impl_;
 };
 
 } // namespace asiolink