On Fri, 28 Feb 2025 14:48:43 GMT, Mark Sheppard <mshep...@openjdk.org> wrote:
>> serhiysachkov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8281511: update Copyright header
>
> I think the checkTime from the base class Tests, should be reviewed in the
> context of the reported failures, where some of the actual timeouts
> registered are 10699 , 16926, 17926
>
> The timeout check is
>
> /* check the time got is within 50% of the time expected */
> public static void checkTime (long got, long expected) {
> checkTime(got, expected, expected);
> }
>
> /* check the time got is between start and end, given 50% tolerance */
> public static void checkTime(long got, long start, long end) {
> dprintln("checkTime: got = " + got + " start = " + start + " end = "
> + end);
> long upper = end + (end / 2);
> long lower = start - (start / 2);
> if (got > upper || got < lower) {
> throw new RuntimeException("checkTime failed: got " + got
> + ", expected between " + start + " and " + end);
> }
> }
>
> which uses arbitrary upper bounds and lower bounds.
>
> If a timeout is set at 5000 milliseconds then the expectation is that, should
> the timeout expire and a SocketTimeoutException is thrown, then the actual
> elapsed time will be equal to, or greater than, the specified timeout value
> passed to setSoTimeout. The minimum timeout is 5000 milliseconds, that is the
> lower bound for any timeout expiry check. Any earlier return would indicate a
> spurious wakeup by the OS.
>
> In the checkTime a lower bound of less than the expected timeout is
> tolerated, and then an upper bound of an additional 50% is utilised as a
> check. Looking at the reported failures scenarios the actual elapsed time is
> significantly greater than the (timeout + 50%) value.
>
> It is proffered that in reality no upper bound, other than > timeout value
> should be imposed on the timeout check.
I see my comment and @msheppar 's crossed each others. Whatever you decide to
do here please fix the exception message to reflect the actual check that is
being made.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/23840#issuecomment-2691159392
