Request for Review: 7094995: test/java/util/zip/ZipFile/ClearStaleZipFileInputStreams.java causes continuous GC in agentvm mode

[Chris Hegarty]

This change does look good.

Not directly related to your changes Neil, and I know we swallow 
InterruptedException in many tests, but does it make sense to set 
keepRunning = false if the GcInducingThread is interrupted?

jtreg will use Thread.interrupt when trying to "cleanup" after the test 
has exited. I know in regular execution this should never happen, I'm 
just thinking about the case where some other bug is lurking, and also 
what is the general practice to handling InterruptedException in these 
tests? I really don't like that we ignore it when jtreg uses it for cleanup.

-Chris.

On 11/22/11 04:16 PM, Neil Richards wrote:
> On Tue, 2011-11-22 at 15:49 +0000, Alan Bateman wrote:
>> On 22/11/2011 11:51, Neil Richards wrote:
>>> Hi all,
>>>
>>> I've created a webrev [1] to address the problem reported in bug
>>> 7094995, where the ClearStaleZipFileInputStreams testcase leaves a
>>> lingering GC inducing thread running after the test ends (when run in
>>> agentvm mode), as spotted by Alan and mentioned in another conversation
>>> [2].
>>>
>>> It modifies the testcase so the main thread tells the GcInducingThread
>>> to shut down, and waits for it to do so (using Thread.join()) before
>>> exiting.
>>>
>>> I've also converted the testcase's use of ZipFile, ZipOutputStream&
>>> FileOutputStream to use ARM (for greater clarity).
>>>
>>> Please let me know your thoughts on this change,
>>>
>> Thanks for coming back to this test. It looks okay to me except I would
>> change the notify to notifyAll (I assume you decided to use wait/notify
>> rather than a volatile flag to avoid waiting for the sleep to complete).
>>
>> -Alan.
>
> Hi Alan,
> Thanks for taking a look at this change.
>
> You're right, I switched to using wait/notify so the GC'ing thread will
> stop waiting as soon as it's been told to shut down.
>
> Changing the notify to notifyAll is harmless enough, and makes it (even)
> clearer that the GC'ing thread will definitely be notified by the call.
>
> I've uploaded a webrev with this amendment [1].
>
> Regards, Neil
>
> [1] http://cr.openjdk.java.net/~ngmr/7094995/webrev.01/
>