duration_cast in chrono_time_traits.hpp prone to overflow, stopping io_service

[mattxg@…]

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?

[anonymous]

Three obvious glitches in my submission:

1) Sorry about the formatting problems. The initial example should be:

// 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){});

and the "Super short solution" should be:

{
  const int64_t 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;
}

Note: I haven't just fixed the formatting, I've included an actual type for the gcd variable.

2) I linked to the wrong function in ffmpeg. An actual example of rescaling while guarding against overflow would be the function av_rescale_rnd here:

​http://ffmpeg.org/doxygen/0.6/mathematics_8c-source.html

3) Obviously, I didn't mean "8.64e+13 quadrillion", I meant simply "8.64e+13".

[chris_kohlhoff]

Fixed on 'develop' in ​92a53bd3cc7fdf75b0d2eb5ce68a41993852d5f3.

Merged to 'master' in ​4e1e7d731fcc5c0104567856de476f7ce8806d72.