[Webkit-unassigned] [Bug 157937] WTF::Condition::waitFor() will time out immediately for relativeTimeout values with very large tick counts

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=157937

Filip Pizlo <fpizlo at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fpizlo at apple.com

--- Comment #2 from Filip Pizlo <fpizlo at apple.com> ---
(In reply to comment #0)
> As discussed in <https://bugs.webkit.org/show_bug.cgi?id=157924>,
> WTF::Condition::waitFor() times out immediately if given a relativeTimeout
> of std::chrono::milliseconds::max(), due to two signed integer overflow bugs
> conspiring against us in Condition::absoluteFromRelative().
> 
> The first happens in this comparison:
> 
>         if (relativeTimeout > Clock::duration::max()) {
> 
> std::chrono::duration converts the operands of its inequality operators to
> the type common to both durations (using std::common_type) before performing
> the comparison. In this case that's nanoseconds, and converting
> milliseconds::max() to nanoseconds will overflow since they both use the
> same underlying data type.

Wow!  I did not know about this behavior.  That's so awkward!  I don't think I would have been so enthusiastic about using std::chrono if I had known how overflow-prone it was.

> 
> The second happens on this line, for the same reason, except this time the
> conversion is explicit:
> 
>         Clock::duration myRelativeTimeout =
>             std::chrono::duration_cast<Clock::duration>(relativeTimeout);
> 
> Since the check that was supposed to protect us from overflowing itself
> overflowed, we now have a negative relative timeout.

Dang, that's too funny.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160522/ce4e8089/attachment-0001.html>