On Mon, 12 Jan 2026 06:05:43 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 six additional > commits since the last revision: > > - merge latest from master branch > - Mark's suggestion - move the duration check to separate method > - merge latest from master branch > - 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 thanks for taking the feedback my openjdk status has yet to be re-asserted but, FWIW ... LGTM ------------- Marked as reviewed by msheppar (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/28919#pullrequestreview-3650507206
