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

Infinite loop in ZipOutputStream.close()

    Details

      Description

      FULL PRODUCT VERSION :
      openjdk version "1.8.0_151"
      OpenJDK Runtime Environment (build 1.8.0_151-b12)
      OpenJDK 64-Bit Server VM (build 25.151-b12, mixed mode)

      ADDITIONAL OS VERSION INFORMATION :
      Linux 2.6.32-696.13.2.el6.x86_64 #1 SMP Thu Oct 5 21:22:16 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

      A DESCRIPTION OF THE PROBLEM :
      ZipOutputStream.close() has the potential for falling into an infinite loop. We've seen this stall out request processing theads on our production tomcat server.

      Our app has code similar to

      void generateZipFileResponse(OutputStream clientOut) {
          try(ZipOutputStream zos = new ZipOutputStream(clientOut);
               PrintWriter pw = new PrintWriter(zos)) {

               // start zip entry
               zos.putNextEntry(new ZipEntry("entry.txt"));

               // generate the entry contents
               generateEntry(pw);
               
               // close entry
               zos.closeEntry();
          }
      }

      Sometimes, when the client disconnects or the socket write times out, the underlying output stream is closed before the zip file is completely written. We've seen this cause the ARM block call of ZipOutputStream.close() to fall into an infinite loop

      A stack trace of the stuck threads looks like

         java.lang.Thread.State: RUNNABLE
      at java.util.zip.Deflater.deflateBytes(Native Method)
      at java.util.zip.Deflater.deflate(Deflater.java:444)
              - locked <0x000000040b42b308> (a java.util.zip.ZStreamRef)
      at java.util.zip.Deflater.deflate(Deflater.java:366)
      at java.util.zip.DeflaterOutputStream.deflate(DeflaterOutputStream.java:251)
      at java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:255)
      at java.util.zip.ZipOutputStream.finish(ZipOutputStream.java:360)
      at java.util.zip.DeflaterOutputStream.close(DeflaterOutputStream.java:238)
      at java.util.zip.ZipOutputStream.close(ZipOutputStream.java:377)
              ...

      I cannot reproduce this issue outside our production machines, however I believe I know what is happening.

      ZipOutputStream.closeEntry() will be called 3 times by generateZipFileResponse() when the underlying SocketOutputStream is closed while it is executing. Since closeEntry() writes to the underlying closed SocketOutputStream, an exception will be generated during the first explicit call. However, the ARM block guarantees two calls to ZipOutputStream.close() which calls closeEntry() because the first call has not completed successfully

      This while loop inside closeEntry() is never terminating

                      def.finish();
                      while (!def.finished()) {
                          deflate();
                      }

      DeflaterOutputStream.deflate() looks like

          protected void deflate() throws IOException {
              int len = def.deflate(buf, 0, buf.length);
              if (len > 0) {
                  out.write(buf, 0, len);
              }
          }

      If Deflater.deflate() returns 0 but doesn't update Deflater.finished to be true, no write will be made to the underlying SocketOutputStream and the loop in ZipOutputStream will never terminate

      Deflater.deflate() ultimately calls Deflater.deflateBytes which it turns out, returns 0 on certain occasions. From Deflater.c


              res = deflate(strm, finish ? Z_FINISH : flush);
              (*env)->ReleasePrimitiveArrayCritical(env, b, out_buf, 0);
              (*env)->ReleasePrimitiveArrayCritical(env, this_buf, in_buf, 0);

              switch (res) {
              case Z_STREAM_END:
                  (*env)->SetBooleanField(env, this, finishedID, JNI_TRUE);
                  /* fall through */
              case Z_OK:
                  this_off += this_len - strm->avail_in;
                  (*env)->SetIntField(env, this, offID, this_off);
                  (*env)->SetIntField(env, this, lenID, strm->avail_in);
                  return len - strm->avail_out;
              case Z_BUF_ERROR:
                  return 0;

      It turns out, zlib deflate() can return Z_BUF_ERROR under one condition that doesn't mean the output-buffer-full/input-buffer-empty. From deflate.c

          /* User must not provide more input after the first FINISH: */
          if (s->status == FINISH_STATE && strm->avail_in != 0) {
              ERR_RETURN(strm, Z_BUF_ERROR);
          }
          
      So here is what I think is happening:
      1. The underlying SocketOutputStream is closed
      2. The next write(s) to the PrintWriter generate IOExceptions whenever the DeflaterOutputStream flushes it's buffer, but these are discarded by PrintWriter. The last write leaves the Deflater part way through consuming it's input buffer
      3. The explicit ZipOutputStream.closeEntry() sets Deflater.finish to true, calls Deflater.deflateBytes for one compression round which ultimately calls deflate() with Z_FINISH as the flush mode. This results in the internal zlib s->status being FINISH_STATE
      4. ZipOutputStream.closeEntry() attempts to call write() on the underlying SocketOutputStream which generates an IOException
      5. The ARM block calls PrintWriter.close() which ultimately calls ZipOutputStream.close() which calls ZipOutputStream.closeEntry() again
      6. ZipOutputStream.closeEntry() once again enters the loop until finished, but the input buffer is still not completely consumed.
      7. This time, calls to zlib's deflate() return Z_BUF_ERROR which results in no update to Deflater.finished and a return of 0 from Deflater.deflateBytes()
      8. Since Deflater.deflateBytes() returned 0, DeflaterOutputStream.deflate() never attempts to write to the underlying closed SocketOutputStream
      9. DeflaterOutputStream.deflate() returns successfully, but Deflater.finished is still false so the loop in ZipOutputStream.closeEntry() continues forever

      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      I have not been able to reproduce this issue outside of our production environment.


      REPRODUCIBILITY :
      This bug can be reproduced often.

      CUSTOMER SUBMITTED WORKAROUND :
      The only workaround I know of is to ensure ZipOutputStream.closeEntry() is only called once, since the first call is guaranteed to complete with an IOException.

      This can be done for single entry zip files by omitting the explicit call to ZipOutputStream.closeEntry() and allowing ZipOutputStream.close() to handle it.

      However, for multi-entry zip files, there is no way to guarantee a single call to ZipOutputStream.closeEntry()

        Attachments

          Activity

            People

            • Assignee:
              sherman Xueming Shen
              Reporter:
              webbuggrp Webbug Group
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated: