On Fri, 19 Dec 2025 07:41:09 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 continues to pass with this change and a te... Thank you for the review, Mark. > BUT I think it possible to simply the main test logic a bit. The Object > wait/notify could be replaced by a CountDownLatch. That's a good idea. I have updated the PR to use a CountDownLatch. The test continues to pass with this change. > one could debate if the temporal checks are necessary, as they cannot be > guaranteed. Given the motivation of this original test, as stated in its `@summary`, I think we should continue testing that the callback does indeed fire within reasonable amount of time, depending on the configured `java.rmi.dgc.leaseValue` value. ------------- PR Comment: https://git.openjdk.org/jdk/pull/28919#issuecomment-3709647389
