I like this suggestion. David's most recent webrev with while(true) loop could
easily be extended to create linked list of reference arrays in each loop. ie.
Object[] chain = null;
while (wr.get() != null) {
try {
Object[] allocate = new Object[1000000];
allocate[0] = chain;
chain = allocate;
} catch( OutOfMemoryError ) {
chain = null;
}
System.gc();
Thread.sleep(100);
}
Mike
On Jan 31 2013, at 05:47 , Peter Levart wrote:
> Hi David,
>
> You could combine the B and C in the following pattern:
>
> - Create a WeakReference to the observed object;
> - Execute the potentially "leaky" operation;
> - Trigger gc with after check of WeakReference. Maybe a couple of times.
> - If that does not clear the reference, fill-in heap until OOME is thrown and
> after that check the reference.
>
> Regards, Peter
>
> On 01/31/2013 10:40 AM, David Buck wrote:
>> Hi!
>>
>> I was curious to see what others have done in the past and took a look at
>> about 15 different testcases for memory leaks in the jdk tree and basically
>> found 3 patterns:
>>
>> Pattern A: Use a loop that would exercise the relevant code for a finite
>> number of runs. If an OOME is not thrown, the test passes. In order for the
>> test to finish quickly, most people seem to specify a very small Xmx to
>> limit the number of iterations needed. (example:
>> java/util/concurrent/BlockingQueue/PollMemoryLeak.java)
>>
>> Pattern B: Code similar to what I have submitted for this review that tries
>> to leak one object, explicitly triggers a GC event, and then confirms that
>> the WeakReference was nulled out. One thing to note is that in every case I
>> found of this the author chose to run multiple GC events, usually somewhere
>> between 5 to 10000 iterations. (example:
>> javax/management/Introspector/ClassLeakTest.java)
>>
>> Pattern C: These cases used a WeakReference like Pattern B above, but
>> instead of depending on an explicit call to System.gc(), would intentionally
>> fill the heap until an OOME is thrown to trigger at least 1 "full" gc event.
>> (example:
>> java/rmi/server/UnicastRemoteObject/unexportObject/UnexportLeak.java)
>>
>> Patterns A and B were most common (with pattern B being slightly more
>> common), and I only found Pattern C 2 times
>>
>> Strictly speaking, all 3 patters are problematic as they each depend on
>> undefined behavior in some way or another. Patterns A and C require the JVM
>> to handle OOMEs somewhat gracefully (Pattern A when we fail, Pattern C when
>> we pass), and pattern B requires an explicit call to System.gc to collect an
>> arbitrary object.
>>
>> The fact that we do not routinely see false positives from these cases
>> indicates that in practice, they all seem to work OK for the time being.
>>
>> After thinking about this for a bit, I actually really like pattern C. Even
>> though the JVM is not technically guaranteed to gracefully handle all OOME
>> conditions, we try very hard to be as robust as possible in the face of
>> Java-heap OOME. Again, the fact that we do not routinely see false positives
>> from these tests proves that in practice, this works very well. Even if an
>> otherwise "correct" change (say in the JVM) resulted in Pattern C suddenly
>> failing, it is something we would almost certainly want to investigate as it
>> would indicate that the robustness of the JVM has somehow regressed.
>>
>> I plan to modify my testcase to follow pattern C and resubmit for review. If
>> anyone has any other ideas or comments, I would be grateful for your input.
>>
>> Cheers,
>> -Buck
>>
>> On 2013/01/30 22:32, David Buck wrote:
>>> Hi Alan!
>>>
>>> Thank you for your help.
>>>
>>> TL;DR version:
>>>
>>> Strictly speaking (going only by the specifications), this could give us
>>> false positives, but I believe this is VERY unlikely to actually happen
>>> in real life.
>>>
>>> Long version:
>>>
>>> Yes, I gave this some thought myself. For example, on JRockit, if the
>>> object were in old space and System.gc() only did a young collection
>>> (the default behavior for JRockit), this test would result in a false
>>> positive. In fact, as the JVM is allowed by the specification to
>>> completely ignore explicit GC calls, we could never guarantee that we
>>> would the WeakReference would always get nulled out.
>>>
>>> That said, in pactice this works very well for both HotSpot and JRockit.
>>> Every scenario I have tried it out on (with both JVMs) has provided the
>>> expected result every single time (i.e. failing when expected; never
>>> resulting in false positive otherwise). It seems that both of Oracle's
>>> JVMs as currently implemented are very unlikely to run into any issues
>>> here. Marking the test cases as "othervm" also helps to remove most edge
>>> case scenarios where you could still somehow imagine this failing. (For
>>> example, on a JRockit-like JVM, other tests running concurrently could
>>> trigger a gc in the middle of this test resulting in the HashMap and its
>>> contents being promoted to old space and the null reference not being
>>> cleared during the call to System.gc() as expected.)
>>>
>>> One option would be to mark this as a manually-run test if we wanted to
>>> be extra cautious. What do you think?
>>>
>>> > Minor nit, should be WeakReference<Object> to avoid the raw type.
>>>
>>> I will update the webrev once we have decided what (if anything) to do
>>> regarding the risk of false positives.
>>>
>>> Cheers,
>>> -Buck
>>>
>>> On 2013/01/30 22:09, Alan Bateman wrote:
>>>> On 29/01/2013 23:36, David Buck wrote:
>>>>> Hi!
>>>>>
>>>>> This is a review request to add only the test case for the following
>>>>> OracleJDK issue :
>>>>>
>>>>> [ 7042126 : (alt-rt) HashMap.clone implementation should be
>>>>> re-examined ]
>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7042126
>>>>>
>>>>> * please note: I just marked the bug as "public" this morning, so
>>>>> there will be a short delay before the above link is available.
>>>>>
>>>>> The issue (root cause) is not in OpenJDK (i.e. the problem was
>>>>> OracleJDK specific), but the test case is valid for any Java SE
>>>>> implementation so it should go into OpenJDK so we can prevent a
>>>>> similar issue from ever happening in both releases moving forward. The
>>>>> test case simply helps ensure that the contents of a HashMap are not
>>>>> leaked when we clone the HashMap.
>>>>>
>>>>> webrev:
>>>>> [ Code Review for jdk ]
>>>>> http://cr.openjdk.java.net/~dbuck/7042126/webrev.00/
>>>> How robust is this test? I'm concerned that it might fail intermittently
>>>> if the System.gc doesn't immediately GC the now un-references entries in
>>>> hm.
>>>>
>>>> Minor nit, should be WeakReference<Object> to avoid the raw type.
>>>>
>>>> -Alan.
>
