OPERATING SYSTEM(S):
All

FULL JDK VERSION(S):
All

DESCRIPTION:

The API doc for java.util.zip.ZipFile.close() states the following:

   "Closing this ZIP file will close all of the input streams
    previously returned by invocations of the getInputStream method."

However, the test below shows that this is not actually the case. This leads to a native memory leak if the input streams are not closed explicitly, because the native buffer created for each ZipEntry is never freed.

REPRODUCTION INSTRUCTIONS:

1. Create a zipfile "ziptester.zip", containing a single file,
   "dummyfile.txt".
2. Compile and run the testcase at the bottom of this report.

The testcase repeatedly opens the zipfile and grabs an input stream for the entry inside. It can be run in fours ways, each cleaning up to a different extent:

   Usage: java ZipTester 0|1|2|3
   Options: 3 = close ZipFile and InputStream after each iteration
            2 = close InputStream, but not ZipFile after each iteration
            1 = close ZipFile, but not InputStream after each iteration
            0 = don't close ZipFile or InputStream after each iteration

With option 3, there is no leak.

With option 2 there is also no leak, because we are closing the InputStream explicitly and the ZipFile itself gets cleaned up by finalization (ZipFile.finalize() calls ZipFile.close()).

Options 1 and 0 demonstrate the problem. Even if the ZipFile is closed (option 1), the InputStream is never closed and we get a native leak. With option 0, the ZipFile gets cleaned up by finalization, but the input stream never gets closed.

The leak can be observed easily on Solaris 9/10 using libumem.so. For example, after running with option 1 for just over 1,000,000 iterations, the native heap has grown to ~450MB and libumem finds the following libzip.so leaks:

   CACHE LEAKED BUFCTL CALLER
   08077710 1055651 086c96b8 libzip.so`newEntry+0x1f
   08077710 4986 080ac9e8 libzip.so`newEntry+0x1f
   08077710 4348 084bcca8 libzip.so`newEntry+0x1f
   08077710 4 080ac718 libzip.so`newEntry+0x1f
   08074710 4984 081eaa10 libzip.so`newEntry+0x179
   08074710 4347 084b9a38 libzip.so`newEntry+0x179
   08074710 1055090 086cff48 libzip.so`newEntry+0x179

The stacks for each allocation look similar to this:

   > 086c96b8::bufctl -v
      ADDR BUFADDR TIMESTAMP THREAD
                      CACHE LASTLOG CONTENTS
   86c96b8 86c7510 364f453cdebc 2
                    8077710 0 0
          libumem.so.1`umem_cache_alloc_debug+0x16c
          libumem.so.1`umem_cache_alloc+0x15c
          libumem.so.1`umem_alloc+0x3f
          libumem.so.1`malloc+0x23
          libzip.so`newEntry+0x1f
          libzip.so`ZIP_GetEntry+0x8a
          libzip.so`Java_java_util_zip_ZipFile_getEntry+0xa7
          0xfb24d082

With the 32-bit Solaris JRE, the test eventually dies with a native OutOfMemoryError after about 7,000,000 iterations:

   java.lang.OutOfMemoryError: requested 344 bytes for
   vframeArray::allocate. Out of swap space?

The native buffers are allocated by the private native getEntry() call in ZipFile.getInputStream():

   ...
   synchronized (this) {
      ensureOpen();
--> jzentry = getEntry(jzfile, name, false);
      if (jzentry == 0) {
         return null;
      }

   in = new ZipFileInputStream(jzentry);

   }
   ...

The native buffer is released by ZipFileInputStream.close():

   synchronized (ZipFile.this) {
      if (jzentry != 0 && ZipFile.this.jzfile != 0) {
--> freeEntry(ZipFile.this.jzfile, jzentry);
         jzentry = 0;
      }
   }

So if close() is never called, the buffer is never freed.

Expected behaviour:
If a ZipFile instance is closed, all the input streams obtained via ZipFile.getInputStream() should also be closed as per the API doc, and there should be no native leak. Even if the ZipFile is never closed explicitly, there should not be a leak so long as finalization keeps up, because the ZipFile finalizer should take care of closing the input streams.

Actual behaviour:
ZipFileInputStreams obtained via ZipFile.getInputStream() are not closed by ZipFile.close(), which contradicts the API doc.

Resolution:
Either the API doc needs to be corrected, or the ZipFile.close() implementation needs to be fixed such that the input streams do in fact get closed.


TESTCASE SOURCE:

----------------------------------------------------------------------
ZipTester.java
----------------------------------------------------------------------
import java.util.zip.*;
import java.io.*;

public class ZipTester {
    public static void main (String args[]) {
        if (args.length != 1) {
            System.out.println("Usage: java ZipTester 0|1|2|3");
            System.out.println("Options: 3 = close ZipFile and InputStream after each iteration");
            System.out.println(" 2 = close InputStream, but not ZipFile after each iteration");
            System.out.println(" 1 = close ZipFile, but not InputStream after each iteration");
            System.out.println(" 0 = don't close ZipFile or InputStream after each iteration");
            System.exit(1);
        }
        int count = 0;
        while (true) {
            try {
                ZipFile zf = new ZipFile(new File("ZipTester.zip"));
                ZipEntry ze = zf.getEntry("dummyfile.txt");
                InputStream is = zf.getInputStream(ze);

                if (args[0].equals("1")) {
                    zf.close();
                }

                if (args[0].equals("2")) {
                    is.close();
                }

                if (args[0].equals("3")) {
                    is.close();
                    zf.close();
                }
            } catch (Exception e) {
                e.printStackTrace();
            }
            count++;

            if (count % 10000 == 0) {
                System.out.println("Opened zipfile " + count + " times...");
                System.gc(); // To keep the finalization rate up as much as possible to effectively remove it from the picture
            }
        }
    }
}
----------------------------------------------------------------------