Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-6907252

ZipFileInputStream Not Thread-Safe

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: P3
    • Resolution: Fixed
    • Affects Version/s: 6u17, 9
    • Fix Version/s: 9
    • Component/s: core-libs
    • Subcomponent:
    • Resolved In Build:
      b88
    • CPU:
      x86
    • OS:
      windows_xp

      Backports

        Description

        FULL PRODUCT VERSION :
        java version "1.6.0_17"
        Java(TM) SE Runtime Environment (build 1.6.0_17-b04)
        Java HotSpot(TM) Client VM (build 14.3-b01, mixed mode, sharing)

        ADDITIONAL OS VERSION INFORMATION :
        Microsoft Windows
        Version 5.1 (Build 2600.xpsp_sp3_gdr.090804-1435 : Service Pack 3)

        A DESCRIPTION OF THE PROBLEM :
        Using an InputStream of a ZIP file entry seems not thread safe, and may even crash the VM.
        Below you'll find two crash reports. The first one is from my test run, the second one from the real issue we've encountered. It happens when reading an animating an animated GIF as a resource from a JAR file. Notice that the affected code is the AWT ImageFetcher.

        STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
        See test program. It's timing issue, so you may have to play with it for a while.

        I think the access violation happens at the following line, while accessing csize of a (already closed?) NULL entry, in function ZIP_Read:
        zip_util.c:1081
            jlong entry_size = (entry->csize != 0) ? entry->csize : entry->size;


        EXPECTED VERSUS ACTUAL BEHAVIOR :
        EXPECTED -
        The exception that happens most of the time is quite good.

        java.io.EOFException: Unexpected end of ZLIB input stream
        at java.util.zip.ZipFile$1.fill(ZipFile.java:227)
        at java.util.zip.InflaterInputStream.read(InflaterInputStream.java:141)
        at java.util.zip.InflaterInputStream.read(InflaterInputStream.java:105)
        at CrashTest.main(CrashTest.java:31)
        ACTUAL -
          From time to time, there's a ZipException instead of the EOFException, with varying, less helpful, messages.

        java.util.zip.ZipException: invalid literal/length code
        at java.util.zip.InflaterInputStream.read(InflaterInputStream.java:147)
        at java.util.zip.InflaterInputStream.read(InflaterInputStream.java:105)
        at CrashTest.main(CrashTest.java:31)

        Other messages are:
            oversubscribed dynamic bit lengths tree
            invalid stored block lengths
            invalid block type

        The real problem is that the issue makes the VM crash some times - refer stack trace.

        ERROR MESSAGES/STACK TRACES THAT OCCUR :
        Attached seperatly

        REPRODUCIBILITY :
        This bug can be reproduced rarely.

        ---------- BEGIN SOURCE ----------
        Any JAR with any file should do. Just make the entry big enough for the timer to hit the reading loop.

        import java.io.EOFException;
        import java.io.File;
        import java.io.IOException;
        import java.io.InputStream;
        import java.util.Timer;
        import java.util.TimerTask;
        import java.util.zip.ZipEntry;
        import java.util.zip.ZipFile;

        public class CrashTest {
            public static void main(final String[] args) throws Exception {
                final Timer timer = new Timer();
                while (true) {
                    final ZipFile zip = new ZipFile(new File("test.jar"));
                    try {
                        final ZipEntry entry = zip.getEntry("test.gif");
                        final InputStream in = zip.getInputStream(entry);
                        try {
                            timer.schedule(new TimerTask() {
                                @Override
                                public void run() {
                                    try {
                                        in.close();
                                    } catch (final IOException ex) {
                                        ex.printStackTrace(System.err);
                                    }
                                }
                            }, 10);
                            int c;
                            do {
                                c = in.read();
                            } while (c != -1);
                        }
                        finally {
                            in.close();
                        }
                    }
                    catch (final EOFException ex) {
                        // "Unexpected end of ZLIB input stream"
                    }
                    catch (final Exception ex) {
                        ex.printStackTrace(System.err);
                    }
                    finally {
                        zip.close();
                    }
                }
            }
        }
        ---------- END SOURCE ----------
        1. CrashTest.java
          2 kB
          Sean Coffey
        2. ZipEntryFreeTest.java
          3 kB
          Sean Coffey

          Issue Links

            Activity

            Hide
            coffeys Sean Coffey added a comment -
            No check is made in ZIP_Read function to test if the underlying jzentry passed in has been freed. An erroneous thread can easily close out the stream while the ZipInputStream is being read. The ZIP_Close function tests for a reference count on the ZipFile itself but that's of no help in this scenario and freeing up of the native structure continues. We need to look at the locking system in use and its coverage.

            One suggested fix for this scenario :

            @@ -1286,9 +1297,17 @@
             jint
             ZIP_Read(jzfile *zip, jzentry *entry, jlong pos, void *buf, jint len)
             {
            - jlong entry_size = (entry->csize != 0) ? entry->csize : entry->size;
            +
            + jlong entry_size;
                 jlong start;

            + if (entry == NULL) {
            + zip->msg = "ZIP_Read: invalid jzentry encountered";
            + return -1;
            + }
            +
            + entry_size = (entry->csize != 0) ? entry->csize : entry->size;
            +

            Another option that will work would be to synchronize the read and close methods within ZipFile$ZipFileInputStream
            Show
            coffeys Sean Coffey added a comment - No check is made in ZIP_Read function to test if the underlying jzentry passed in has been freed. An erroneous thread can easily close out the stream while the ZipInputStream is being read. The ZIP_Close function tests for a reference count on the ZipFile itself but that's of no help in this scenario and freeing up of the native structure continues. We need to look at the locking system in use and its coverage. One suggested fix for this scenario : @@ -1286,9 +1297,17 @@  jint  ZIP_Read(jzfile *zip, jzentry *entry, jlong pos, void *buf, jint len)  { - jlong entry_size = (entry->csize != 0) ? entry->csize : entry->size; + + jlong entry_size;      jlong start; + if (entry == NULL) { + zip->msg = "ZIP_Read: invalid jzentry encountered"; + return -1; + } + + entry_size = (entry->csize != 0) ? entry->csize : entry->size; + Another option that will work would be to synchronize the read and close methods within ZipFile$ZipFileInputStream
            Hide
            coffeys Sean Coffey added a comment -
            reproducer attached. Copy over an rt.jar file to local directory to test.
            Show
            coffeys Sean Coffey added a comment - reproducer attached. Copy over an rt.jar file to local directory to test.
            Hide
            coffeys Sean Coffey added a comment -
            Better reproducer attached (ZipEntryFreeTest). Fix needed is to ensure the underlying jzentry resource hasn't been freed while the stream is being closed. This should resolve many of the ZIP_Read+0xc type crashes randomly seen in the JDK. e.g.

            C [libzip.so+0x8424] ZIP_Read+0xc
            C [libzip.so+0x8344] Java_java_util_zip_ZipFile_read+0x9c
            J 83 java.util.zip.ZipFile.read(JJJ[BII)I (0 bytes) @ 0xffffffff67c047e0 [0xffffffff67c04660+0x180]
            J 78 C1 java.util.zip.ZipFile$ZipFileInputStream.read([BII)I (131 bytes) @ 0xffffffff60c2d914 [0xffffffff60c2d380+0x594]
            J 77 C1 java.util.zip.ZipFile$ZipFileInputStream.read()I (25 bytes) @ 0xffffffff60c2cebc [0xffffffff60c2cd00+0x1bc]


            Show
            coffeys Sean Coffey added a comment - Better reproducer attached (ZipEntryFreeTest). Fix needed is to ensure the underlying jzentry resource hasn't been freed while the stream is being closed. This should resolve many of the ZIP_Read+0xc type crashes randomly seen in the JDK. e.g. C [libzip.so+0x8424] ZIP_Read+0xc C [libzip.so+0x8344] Java_java_util_zip_ZipFile_read+0x9c J 83 java.util.zip.ZipFile.read(JJJ[BII)I (0 bytes) @ 0xffffffff67c047e0 [0xffffffff67c04660+0x180] J 78 C1 java.util.zip.ZipFile$ZipFileInputStream.read([BII)I (131 bytes) @ 0xffffffff60c2d914 [0xffffffff60c2d380+0x594] J 77 C1 java.util.zip.ZipFile$ZipFileInputStream.read()I (25 bytes) @ 0xffffffff60c2cebc [0xffffffff60c2cd00+0x1bc]
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/d97306dd54cd
            User: coffeys
            Date: 2015-10-15 08:34:01 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/d97306dd54cd User: coffeys Date: 2015-10-15 08:34:01 +0000
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/d97306dd54cd
            User: lana
            Date: 2015-10-21 22:17:00 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/d97306dd54cd User: lana Date: 2015-10-21 22:17:00 +0000

              People

              • Assignee:
                coffeys Sean Coffey
                Reporter:
                ndcosta Nelson Dcosta
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Imported:
                  Indexed: