Rewrite GetTickCount64 emulation implementation.

[Lastique]

This is to resolve the possible license violation as the previous implementation has been taken from StackOverflow and was not licensed under the Boost Software License. The new implementation was adopted from Boost.Log:

https://github.com/boostorg/log/blob/1cc577cbf5fae8f55c71c4493a7ef89027bd85dc/src/timestamp.cpp#L66-L86

The legal issue has been raised in:

https://lists.boost.org/Archives/boost/2018/02/241453.php

Fixes https://github.com/boostorg/thread/issues/209.

[ned14]

Unfortunately this PR would be a regression as currently written. The original bug report that I fixed was timers failing on a computer which sleeps for several months at a time. This PR does not detect and directly handle multiple 43 day system timer wraps as the original does.

[Lastique]

I've updated the implementation to mitigate the GetTickCount wraparound problem if the user doesn't call GetTickCount64 for extended periods of time.

[ned14]

Eh, surely a sleeping computer wouldn't call your refresh routine? Thus you get multiple wraps and the timer fires late?

[Lastique]

Does GetTickCount keep going while the machine is sleeping?

[ned14]

Yes

[Lastique]

Oh well. Then if the machine sleeps for too long (more than ~49 days) the algorithm will miss one or more wraparounds. The current implementation doesn't seem to protect against that either, only it returns a bogus 0xFFFFFFFF value (not reliably - if the wrapped tick count doesn't match the condition in https://github.com/boostorg/thread/blob/develop/include/boost/thread/win32/thread_primitives.hpp#L124). Out of the two I'd prefer my result since it at least somehow correlates with reality.

Anyways, let Vicente decide what to do with this PR. I'm fine if it's rejected.

[ned14]

I honestly can't remember whether we specially handle the 32 bit GetTickCount64 emulation wrapping and returning -1. I do remember that we had a very eager bug reporter who did a lot of testing and verified the bug was fixed. Can't remember if that was on 64 bit only, and we threw WinXP to the wind.

Actually, seeing as we've now dropped WinXP as a supported platform, we could just axe the emulation altogether? Vista and later implement GetLastCount64(), including on x86 platforms.

[Lastique]

The special return value is 0xFFFFFFFF, not -1 (note that GetTickCount64 return ULONGLONG, the 64-bit unsigned integer). The only place that calls GetTickCount64 (https://github.com/boostorg/thread/blob/develop/include/boost/thread/detail/platform_time.hpp#L432) does not check for this value. And the genuine GetTickCount64 has no special return values to indicate errors, so checking for 0xFFFFFFFF would be a wrong thing to do anyway.

Actually, seeing as we've now dropped WinXP as a supported platform, we could just axe the emulation altogether?

I'm not aware of Boost.Thread dropping support for XP. Boost in general certainly didn't drop XP.

If Boost.Thread does not support pre-Vista then I agree, calling GetTickCount64 directly is more preferable.

[viboes]

Thanks to both for working on this. Sorry, I know nothing about Windows.

IIUC neither the current code nor the proposed patch are perfect solutions to the problem, isn't it? Could we state clearly what are the advantages/limitations of each approach? Niall, do you believe that Andrey solution is not satisfactory?

IIUC, we use the Windows ::GetTickCount64 if available, isn't it? In the absence of 64 bits counters, do we really need an emulation? Couldn't we just have a restriction? It is too late?

[ned14]

WinXP EOLed in 2014. WinVista EOLed last year, in 2017.

If you feel that continuing to support WinXP is excessive, both of us agree that the GetTickCount64 emulation function ought to be removed entirely.

So decide on that first, then we go from there.

[Lastique]

IIUC neither the current code nor the proposed patch are perfect solutions to the problem, isn't it? Could we state clearly what are the advantages/limitations of each approach?

Neither version works correctly if the machine sleeps for prolonged time. The PR version may miss one or more 32-bit wraparounds (which is ~49.7 days) in this case. E.g. if the machine sleeps for 50 days and then calls GetTickCount64, the result will change as if only ~0.3 days passed. The current git develop code may miss one or more 28-bit wraparounds (~3.1 days) or may return a bogus value of 0xFFFFFFFF regardless of the current time in this case.

Aside from machine sleeps, the PR version should work correctly without restrictions (it will automatically refresh its internal state to track 32-bit wraparounds). The current develop code still requires the user to call the emulation code at least every ~6.2 days, otherwise it will either miss one or more 28-bit wraparounds or return 0xFFFFFFFF.

[Lastique]

IIUC, we use the Windows ::GetTickCount64 if available, isn't it?

Yes, both versions do.

[viboes]

Thanks to both.

We need to support any version we support today, and in particular WinXP. Nial, my concern wasn't not about not supporting WinXp, but on not using an emulation for GetTickCount64, but this was surely a bad idea.

I believe that the PR limitations are acceptable. Niall if you think the PR has any major issues, please tell us which. If no one is completely against, I'll merge this PR 25/02/2018.

Vicente

[Kojoley]

I am getting an error on Appveyor that I could not reproduce locally.

.\boost/thread/win32/thread_primitives.hpp(73): error C2143: syntax error: missing ')' before '*'
.\boost/thread/win32/thread_primitives.hpp(73): error C2143: syntax error: missing ';' before '*'
.\boost/thread/win32/thread_primitives.hpp(73): error C2059: syntax error: ')'
.\boost/thread/win32/thread_primitives.hpp(73): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
.\boost/thread/win32/thread_primitives.hpp(74): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
.\boost/thread/win32/thread_primitives.hpp(74): error C2365: 'boost::detail::win32::detail::gettickcount64_t': redefinition; previous definition was 'function'
.\boost/thread/win32/thread_primitives.hpp(73): note: see declaration of 'boost::detail::win32::detail::gettickcount64_t'
.\boost/thread/win32/thread_primitives.hpp(74): error C2146: syntax error: missing ';' before identifier 'gettickcount64'

Update: It looks like Boost.Thread CI is experiencing the same issue.

[Lastique]

See https://github.com/boostorg/thread/pull/214.