Avoid usage of the non monotonic clock System.currentTimeMillis() in favor of System.nanoTime()#6392
Conversation
|
The comment https://github.com/testcontainers/testcontainers-java/pull/6392/files#diff-1cb71d0ebd85bd607c3da0c4e693f353e5d0b9c947a2396ffc32042f3ccbb63eL45 is now incorrect, but a good estimate is hard to guess |
|
Thanks for providing a PR @Nateckert. Is this an issue you have observed in your system while using Testcontainers? Could you please run |
I ran the formatter. I actually don't know if we encounter that issue, but we are currently trying to sanitize our CI and this was a quick fix to a unlikely problem that can be almost impossible to debug (looks random, there is no associated logs, ...), so that's why I wanted to contribute |
| private void waitUntil(Predicate<OutputFrame> predicate, long expiry, int times) throws TimeoutException { | ||
| int numberOfMatches = 0; | ||
| while (System.currentTimeMillis() < expiry) { | ||
| while (System.nanoTime() < expiry) { |
There was a problem hiding this comment.
nanoTime is subject to overflow/wrapping and the addition of an offset may have wrapped.
Whilst this could have happened with mills you wouldn't care for a few more years. nanoTime is relative to a random point in time so it could be a very large number or a very small number, positive or megative
As per javadoc
To compare two nanoTime values
long t0 = System.nanoTime();
...
long t1 = System.nanoTime();
one should use t1 - t0 < 0, not t1 < t0, because of the possibility of numerical overflow.
With the changes after @jtnord's feedback, I managed to update the estimate |
|
Thanks for your contribution, @Nateckert ! |
…favor of System.nanoTime() (testcontainers#6392) Co-authored-by: Eddú Meléndez Gonzales <eddu.melendez@gmail.com>
In Java,
System.currentTimeMillis()is a non-monotonic clock, which means thatmight print a number smaller than zero !
This is due to the fact that the internal clock of computer if not accurate and it is put back on the "right" time thanks to the UDP protocol.
Additional read: https://superuser.com/questions/759730/how-much-clock-drift-is-considered-normal-for-a-non-networked-windows-7-pc
Instead,
System.nanoTime()should be prefer for timing purpose, as it will guarantee that two successive calls to it are greater than zero