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

[Joe Darcy]

Hello,

On 11/22/2011 9:57 PM, David Holmes wrote:
> Hi Neil,
>
> On 22/11/2011 9:51 PM, Neil Richards wrote:
>> 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.
>
> As per the discussion from Alan and Chris, I would code it so that 
> interrupt terminates the thread:
>
>    catch (InterruptedException ie) {
>      return;
>    }
>
>> I've also converted the testcase's use of ZipFile, ZipOutputStream&
>> FileOutputStream to use ARM (for greater clarity).
>
> I think proper use of ARM requires that this:
>
>   66         try (ZipOutputStream zos =
>   67                 new ZipOutputStream(new 
> FileOutputStream(tempZipFile))) {
>
> be written as:
>
>    try (FileOutputStream fos = new FileOutputStream(tempZipFile);
>         ZipOutputStream zos = new ZipOutputStream(fos)) {
>

The more conservative approach is to define one resource variable per 
logical resource even if the one resource is "wrapped" by the second 
one, as in the second example.  This does close the small window of 
vulnerability between when the first constructor call ends and the 
second one completes.  However, if that pattern is used, it is important 
for the close method of the inner resource to be idempotent, as required 
by the java.io.Closeable type but *not* required by java.lang.AutoCloseable.

-Joe