> 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...

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

-------------

Changes:
  - all: https://git.openjdk.org/jdk/pull/28919/files
  - new: https://git.openjdk.org/jdk/pull/28919/files/af9e6502..d086512d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=28919&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=28919&range=00-01

  Stats: 5957 lines in 2014 files changed: 1797 ins; 727 del; 3433 mod
  Patch: https://git.openjdk.org/jdk/pull/28919.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/28919/head:pull/28919

PR: https://git.openjdk.org/jdk/pull/28919

Reply via email to