On Fri, 6 Sep 2024 16:28:36 GMT, Aleksei Efimov <aefi...@openjdk.org> wrote:
> This PR proposes the following changes to address wrong timeout computations > in the `com.sun.jndi.dns.DnsClient`: > - The `DnsClient` has been updated to use a monotonic high-resolution (nano) > clock. The existing `Timeout` test has also been updated to use the nano > clock to measure observed timeout value. > > - The left timeout computation has been fixed to decrease the timeout value > during each retry attempt. A new test, `TimeoutWithEmptyDatagrams`, has been > added to test it. > > - The `DnsClient.blockingReceive` has been updated: > - to detect if any data is received > - to avoid contention with `Selector.close()` that could be called by a > cleaner thread > > - The expected timeout calculation in the `Timeout` test has been updated to > take into account the minimum retry timeout (50ms). Additionally, the max > allowed difference between the observed timeout and the expected one has been > increased from 50% to 67%. Taking into account 50 ms retry timeout decrease > the maximum allowed difference is effectively set to 61%. This change is > expected to improve the stability of the `Timeout` test which has been seen > to fail [intermittentlly](https://bugs.openjdk.org/browse/JDK-8220213). If no > objections, I'm planning to close > [JDK-8220213](https://bugs.openjdk.org/browse/JDK-8220213) as duplicate of > this one. > > JNDI/DNS jtreg tests has been executed multiple times (500+) to check if the > new and the modified tests are stable. No failures been observed (so far?). src/jdk.naming.dns/share/classes/com/sun/jndi/dns/DnsClient.java line 478: > 476: long elapsedMillis = Math.max(1, > 477: TimeUnit.NANOSECONDS.toMillis(end - start)); > 478: timeoutLeft = timeoutLeft - (int) elapsedMillis; Hello Aleksei, should this `int` cast take into account potential integer value overflow? In theory (and probably even in practice depending on what the initial timeout value was configured to), the `elapsedMillis`, I think can be a value greater than `Integer.MAX_VALUE`, in which case this cast to `int` can cause unexpected computation of `timeoutLeft`. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20892#discussion_r1748082435
