Boost C++ Libraries: Ticket #9778: duration_cast in chrono_time_traits.hpp prone to overflow, stopping io_service https://svn.boost.org/trac10/ticket/9778 <p> The following code: </p> <blockquote> <p> <em> Assume "io_service" working and running normally </em></p> </blockquote> <blockquote> <p> boost::asio::steady_timer timer(io_service); timer.expires_from_now(std::chrono::hours(24)); timer.async_wait([](const boost::system::error_code&amp; e){}); </p> </blockquote> <p> will cause the io_service to stop processing further events on Mac OS X and probably causes other serious problems on other platforms. </p> <p> Why? </p> <p> 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(): </p> <blockquote> <p> return ticks() * num / den; </p> </blockquote> <p> 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. </p> <p> 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: </p> <blockquote> <p> int num_events = kevent(kqueue_fd_, 0, 0, events, 128, timeout); </p> </blockquote> <p> resulting in no events ever getting processed again. Bummer. </p> <p> Super short solution with no runtime performance impact... replace the offending line in chrono_time_traits.hpp with: </p> <blockquote> <p> { </p> <blockquote> <p> const gcd = boost::math::static_gcd&lt;period_type::num * Den, period_type::den * Num&gt;::value; const int64_t num_reduced = num / gcd; const int64_t den_reduced = den / gcd; return ticks() * num_reduced / den_reduced; </p> </blockquote> <p> } </p> </blockquote> <p> and include boost/math/common_factor.hpp to make this work. It would probably also be a good idea to replace the </p> <blockquote> <p> if (usec == 0) </p> <blockquote> <p> return 1; </p> </blockquote> </blockquote> <p> comparison in to_usec in boost/asio/detail/timer_queue.hpp with: </p> <blockquote> <p> if (usec &lt;= 0) </p> <blockquote> <p> return 1; </p> </blockquote> </blockquote> <p> 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). </p> <p> A proper solution would require a careful, guarded reduce and scale. See av_reduce here for an example of what that would involve: </p> <blockquote> <p> <a class="ext-link" href="http://ffmpeg.org/doxygen/trunk/rational_8c_source.html"><span class="icon">​</span>http://ffmpeg.org/doxygen/trunk/rational_8c_source.html</a> </p> </blockquote> <p> although I'm not sure this heavy-handed a solution is appropriate, even if the code already exists in boost somewhere. </p> <p> Given the severity of overflow (entire io_service stops responding), maybe some kind of assert condition is warranted? </p> en-us Boost C++ Libraries /htdocs/site/boost.png https://svn.boost.org/trac10/ticket/9778 Trac 1.4.3 anonymous Sat, 15 Mar 2014 04:40:08 GMT <link>https://svn.boost.org/trac10/ticket/9778#comment:1 </link> <guid isPermaLink="false">https://svn.boost.org/trac10/ticket/9778#comment:1</guid> <description> <p> Three obvious glitches in my submission: </p> <p> 1) Sorry about the formatting problems. The initial example should be: </p> <pre class="wiki">// 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&amp; e){}); </pre><p> and the "Super short solution" should be: </p> <pre class="wiki">{ const int64_t gcd = boost::math::static_gcd&lt;period_type::num * Den, period_type::den * Num&gt;::value; const int64_t num_reduced = num / gcd; const int64_t den_reduced = den / gcd; return ticks() * num_reduced / den_reduced; } </pre><p> <strong>Note:</strong> I haven't just fixed the formatting, I've included an actual type for the gcd variable. </p> <p> 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: </p> <blockquote> <p> <a class="ext-link" href="http://ffmpeg.org/doxygen/0.6/mathematics_8c-source.html"><span class="icon">​</span>http://ffmpeg.org/doxygen/0.6/mathematics_8c-source.html</a> </p> </blockquote> <p> 3) Obviously, I didn't mean "8.64e+13 quadrillion", I meant simply "8.64e+13". </p> </description> <category>Ticket</category> </item> <item> <dc:creator>chris_kohlhoff</dc:creator> <pubDate>Mon, 05 May 2014 06:53:30 GMT</pubDate> <title>status, severity changed; resolution set https://svn.boost.org/trac10/ticket/9778#comment:2 https://svn.boost.org/trac10/ticket/9778#comment:2 <ul> <li><strong>status</strong> <span class="trac-field-old">new</span> → <span class="trac-field-new">closed</span> </li> <li><strong>resolution</strong> → <span class="trac-field-new">fixed</span> </li> <li><strong>severity</strong> <span class="trac-field-old">Showstopper</span> → <span class="trac-field-new">Problem</span> </li> </ul> <p> Fixed on 'develop' in <a class="ext-link" href="https://github.com/boostorg/asio/commit/92a53bd3cc7fdf75b0d2eb5ce68a41993852d5f3"><span class="icon">​</span>92a53bd3cc7fdf75b0d2eb5ce68a41993852d5f3</a>. </p> <p> Merged to 'master' in <a class="ext-link" href="https://github.com/boostorg/asio/commit/4e1e7d731fcc5c0104567856de476f7ce8806d72"><span class="icon">​</span>4e1e7d731fcc5c0104567856de476f7ce8806d72</a>. </p> Ticket