On Mon, 5 Jan 2026 09:42:35 GMT, Jaikiran Pai <[email protected]> wrote:
>> Can I please get a review of this test-only change which addresses
>> intermittent failures in
>> `java/rmi/server/Unreferenced/leaseCheckInterval/LeaseCheckInterval.java`?
>>
>> The `@summary` in that test's test definition about what this test does:
>>
>>> @summary When the "java.rmi.dgc.leaseValue" system property is set to a
>>> value much lower than its default (10 minutes), then the server-side
>>> user-visible detection of DGC lease expiration-- in the form of
>>> Unreferenced.unreferenced() invocations and possibly even local garbage
>>> collection (including weak reference notification, finalization, etc.)--
>>> may be delayed longer than expected. While this is not a spec violation
>>> (because there are no timeliness guarantees for any of these garbage
>>> collection-related events), the user might expect that an unreferenced()
>>> invocation for an object whose last client has terminated abnormally
>>> should occur on relatively the same time order as the lease value
>>> granted.
>>
>> In its current form, the test uses a lease expiry of 10 seconds, launches a
>> trivial `java` application which looks up the bound object from the registry
>> and then terminates itself. After launching that trivial java application,
>> the test then waits for 20 seconds, expecting that the
>> `Unreferenced.unreferenced()` callback (upon lease expiry of 10 seconds)
>> will be called within those 20 seconds. This wait intermittently fails
>> because the `Unreferenced.unreferenced()` doesn't get called within those 20
>> seconds.
>>
>> Experiments show that the reason for these intermittent failures is due to
>> the `SelfTerminator` application which does the registry lookup (and which
>> involves connection establishment and communication over a socket) can
>> sometimes take several seconds (5 or more for example). That effectively
>> means that by the time this `SelfTerminator` starts its termination after
>> the lookup, it's already several seconds into the "wait()" in the test.
>>
>> The commit in this PR cleans up the test to more accurately track the
>> duration of how long it took between the lease expiry and the
>> `Unreferenced.unreferenced()` callback to be invoked. Additionally, just to
>> make the test more robust, the maximum expected duration has been increased
>> to 60 seconds instead of 20 seconds. Given the text in the test's summary, I
>> think this increase is still within the expectations of how long it takes
>> for the callback to be invoked after the client has exited abnormally.
>>
>> The test continue...
>
> Jaikiran Pai has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains three additional
> commits since the last revision:
>
> - Mark's review - use CountDownLatch
> - merge latest from master branch
> - 8170896: TEST_BUG:
> java/rmi/server/Unreferenced/leaseCheckInterval/LeaseCheckInterval.java
> failed with unreferenced() not invoked after 20.0 seconds
test/jdk/java/rmi/server/Unreferenced/leaseCheckInterval/LeaseCheckInterval.java
line 111:
> 109: System.err.println("waiting for unreferenced() callback...");
> 110: callbackInvocationLatch.await();
> 111: Instant waitEndedAt = Instant.now();
111 - 126 refactor extract method isWithinExpectedTimeLimit
simpllifies reading of test, and lucidly expresses the logic of the time
evaluation
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28919#discussion_r2664640429
