-
Notifications
You must be signed in to change notification settings - Fork 38
8365398: TEST_BUG: java/rmi/transport/checkLeaseInfoLeak/CheckLeaseLeak.java failing intermittently #46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8365398: TEST_BUG: java/rmi/transport/checkLeaseInfoLeak/CheckLeaseLeak.java failing intermittently #46
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,11 +58,14 @@ | |
| import java.io.*; | ||
| import java.lang.reflect.*; | ||
| import java.rmi.registry.*; | ||
| import java.util.concurrent.CountDownLatch; | ||
| import sun.rmi.transport.*; | ||
|
|
||
| public class CheckLeaseLeak extends UnicastRemoteObject implements LeaseLeak { | ||
| public CheckLeaseLeak() throws RemoteException { } | ||
| public void ping () throws RemoteException { } | ||
| public void ping () throws RemoteException { | ||
| remoteCallsComplete.countDown(); | ||
| } | ||
|
|
||
| /** | ||
| * Id to fake the DGC_ID, so we can later get a reference to the | ||
|
|
@@ -74,6 +77,9 @@ public void ping () throws RemoteException { } | |
| private final static int numberPingCalls = 0; | ||
| private final static int CHECK_INTERVAL = 400; | ||
| private final static int LEASE_VALUE = 20; | ||
| private static final int NO_OF_CLIENTS = ITERATIONS; | ||
| private static final int GOOD_LUCK_FACTOR = 2; | ||
| private static CountDownLatch remoteCallsComplete = new CountDownLatch(NO_OF_CLIENTS); | ||
|
|
||
| public static void main (String[] args) { | ||
| CheckLeaseLeak leakServer = null; | ||
|
|
@@ -113,8 +119,14 @@ public static void main (String[] args) { | |
| jvm.destroy(); | ||
| } | ||
| } | ||
| try { | ||
| remoteCallsComplete.await(); | ||
| System.out.println("remoteCallsComplete . . . "); | ||
| } catch (InterruptedException intEx) { | ||
| System.out.println("remoteCallsComplete.await interrupted . . . "); | ||
| } | ||
| Thread.sleep(NO_OF_CLIENTS * LEASE_VALUE * GOOD_LUCK_FACTOR); | ||
| numLeft = getDGCLeaseTableSize(); | ||
| Thread.sleep(3000); | ||
|
|
||
| } catch(Exception e) { | ||
| TestLibrary.bomb("CheckLeaseLeak Error: ", e); | ||
|
|
@@ -125,8 +137,8 @@ public static void main (String[] args) { | |
| } | ||
| } | ||
|
|
||
| /* numLeft should be 4 - if 11 there is a problem. */ | ||
| if (numLeft > 4) { | ||
| /* numLeft should not be greater than 2 - if 11 there is a problem. */ | ||
| if (numLeft > 2) { | ||
|
Comment on lines
+140
to
+141
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand your comment: "The "(numLeft > 2)" condition which was changed to "(numLeft > 4)" then changed back to "(numLeft > 2)". AFAICS, the original commit has this hunk exactly as you have written here? openjdk/jdk@4b4d0cd#diff-9348ad66aa09ad1a3d437e736b0ec93f9f674ba91864fd15ca59375093386548L128-R141
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This commit changed the condition in line 132/129 of "CheckLeaseLeak.java" from "(numLeft > 2)" to "(numLeft > 4)" Then the following commit changed the condition in line 129/141 from "(numLeft > 4)" back to "(numLeft > 2)"
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but why does it matter for this backport? openjdk/jdk@5a44219 is already in 25u. So openjdk/jdk@4b4d0cd that you are backporting is clean addition on top of it? I am just confused why the merge was even required. |
||
| TestLibrary.bomb("Too many objects in DGCImpl.leaseTable: "+ | ||
| numLeft); | ||
| } else { | ||
|
|
@@ -204,8 +216,9 @@ private static int getDGCLeaseTableSize () { | |
| * objects if the LeaseInfo memory leak is not fixed. | ||
| */ | ||
| leaseTable = (Map) f.get(dgcImpl[0]); | ||
|
|
||
| numLeaseInfosLeft = leaseTable.size(); | ||
| synchronized (leaseTable) { | ||
| numLeaseInfosLeft = leaseTable.size(); | ||
| } | ||
|
|
||
| } catch(Exception e) { | ||
| TestLibrary.bomb(e); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see why you would want to do this, as mainline change introduced this out-of-order. But for backports, you want to stay faithful to the original commit as much as possible. So refrain from making cosmetic changes as you go. If you still want to fix this, you can do a trivial PR against mainline, and backport the follow-up fix. But I think it does not worth the trouble.