RFR: 8339538: Wrong timeout computations in DnsClient [v2]

[Aleksei Efimov]

On Mon, 9 Sep 2024 13:58:51 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:

>> Aleksei Efimov has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - guard against possible integer value overflows
>>  - make startTime a local variable
>
> src/jdk.naming.dns/share/classes/com/sun/jndi/dns/DnsClient.java line 477:
> 
>> 475:                     long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(end - start);
>> 476:                     // Setting the Math.clamp min to 1 ensures that the timeout is decreased
>> 477:                     timeoutLeft = timeoutLeft - Math.clamp(elapsedMillis, 1, Integer.MAX_VALUE);
> 
> I think I'd prefer to calculate the remaining timeout based on the total elapsed time in this loop, as opposed to the time spent in blockingReceive.

Sounds like a right thing to do:  measuring time in the loop should give us better estimation on time DNS client spends waiting on the response after submiting a query (that's how environment property value is defined in [javadoc here](https://docs.oracle.com/en/java/javase/22/docs/api/jdk.naming.dns/module-summary.html)).
I've tried to move `start` and `end` like:

                 do {
+                    long start = System.nanoTime();
<....>
-                    long start = System.nanoTime();
                     gotData = blockingReceive(udpChannel, ipkt, timeoutLeft);
-                    long end = System.nanoTime();
<...>
+                    long end = System.nanoTime();
                     long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(end - start);

As a result the tests showed that the timeout happened with a bit better precision (10th of milliseconds).
I will run more tests and incorporate the suggestions. Thank you.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20892#discussion_r1750402311