On Thu, 22 May 2025 09:59:58 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> We observed a case where the instants returned by `TimeSource.now()` were
>> returned in non-monotonic order. The reason was that sometimes we were using
>> a delay calculated with one `localSource` as an input to a different
>> (updated on another thread) `localSource`. This was confirmed by putting
>> `assert firstNanos + delay == nanos;` under `instant(long, long)`.
>>
>> The fix ensures that we won't accidentally use the incorrect delay by
>> removing the `instant(long, long)` overload, and calculating the delay in
>> the method where it is used.
>>
>> No new test; instrumenting this class for testing would likely double its
>> size.
>
> src/java.net.http/share/classes/jdk/internal/net/http/common/TimeSource.java
> line 97:
>
>> 95: // use localSource if possible to avoid a volatile read
>> 96: if (source.isInWindow(delay)) {
>> 97: return source.instant(nanos);
>
> I would keep the two args intant() method which avoids computing the delay
> twice. Using a local variable `source` in this method should be enough to
> solve the bug.
Hello Daniel, with this proposed change, is there any reason to have the
`localSource` field in this `TimeSource` anymore?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25390#discussion_r2102234265
