[Boost-bugs] [Boost C++ Libraries] #9778: duration_cast in chrono_time_traits.hpp prone to overflow, stopping io_service

Subject: [Boost-bugs] [Boost C++ Libraries] #9778: duration_cast in chrono_time_traits.hpp prone to overflow, stopping io_service
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2014-03-15 04:17:02

#9778: duration_cast in chrono_time_traits.hpp prone to overflow, stopping
io_service
------------------------------+----------------------------
 Reporter:  mattxg@…          |      Owner:  chris_kohlhoff
     Type:  Bugs              |     Status:  new
Milestone:  To Be Determined  |  Component:  asio
  Version:  Boost 1.55.0      |   Severity:  Showstopper
 Keywords:                    |
------------------------------+----------------------------
 The following code:

     // Assume "io_service" working and running normally

     boost::asio::steady_timer timer(io_service);
     timer.expires_from_now(std::chrono::hours(24));
     timer.async_wait([](const boost::system::error_code& e){});

 will cause the io_service to stop processing further events on Mac OS X
 and probably causes other serious problems on other platforms.

 Why?

 std::chrono::hours(24) is 86400000000000 nanoseconds. This value is scaled
 to microseconds in boost/asio/detail/chrono_time_traits.hpp by the
 following line of code in duration_cast():

     return ticks() * num / den;

 where num is 1 million and den is 1 billion. Unfortunately, 8.64e+13
 quadrillion times 1e+6 is 8.64e+19 and overflows a signed 64-bit integer
 (LLONG_MAX is 9.22e+18) giving a negative number. I used 24 hours in the
 example. The real threshold is closer to 2 hours 34 minutes.

 The calling code in to_usec in timer_queue.hpp checks for zero but does
 not check for negative numbers. The negative time is then use in the
 kevent call in kqueue_reactor.ipp:

     int num_events = kevent(kqueue_fd_, 0, 0, events, 128, timeout);

 resulting in no events ever getting processed again. Bummer.

 Super short solution with no runtime performance impact... replace the
 offending line in chrono_time_traits.hpp with:

       {
         const gcd = boost::math::static_gcd<period_type::num * Den,
 period_type::den * Num>::value;
         const int64_t num_reduced = num / gcd;
         const int64_t den_reduced = den / gcd;
         return ticks() * num_reduced / den_reduced;
       }

 and include boost/math/common_factor.hpp to make this work. It would
 probably also be a good idea to replace the

     if (usec == 0)
         return 1;

 comparison in to_usec in boost/asio/detail/timer_queue.hpp with:

     if (usec <= 0)
         return 1;

 However, this solution will only address the scenario where the num and
 den both contain a large common factor (yes, the common case but still
 won't solve everything).

 A proper solution would require a careful, guarded reduce and scale. See
 av_reduce here for an example of what that would involve:

     http://ffmpeg.org/doxygen/trunk/rational_8c_source.html


 although I'm not sure this heavy-handed a solution is appropriate, even if
 the code already exists in boost somewhere.

 Given the severity of overflow (entire io_service stops responding), maybe
 some kind of assert condition is warranted?

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/9778>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.