Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-3392

Java TZlibTransport does not close its wrapper streams upon close()

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.9.3
    • Fix Version/s: 0.10.0
    • Component/s: Java - Library
    • Labels:
      None

      Description

      While setting up a short demo project using the new Java TZlibTransport in 0.9.3, I wrote code like this:

      try (TProtocol proto = new TJSONProtocol(new TZlibTransport(...))) {
        Blog blog = new Blog();
        // set up fields in blog
        proto.write(blog);
      }
      

      However, when I went to look at the file, I found it mostly empty (with only the header bytes). I had a look at the TZlibTransport code, which seems to be a subclass of TIOStreamTransport that wraps its underlying transport using Java inflater/deflater streams:

      public TZlibTransport(TTransport transport, int compressionLevel) {
        transport_ = transport;
        inputStream_ = new InflaterInputStream(new TTransportInputStream(transport_), new Inflater());
        outputStream_ = new DeflaterOutputStream(new TTransportOutputStream(transport_), new Deflater(compressionLevel, false), true);
      }
      

      However, on its close method it only closes the transport and not the underlying streams, forcing users to flush the transport before closing if they really want to write everything:

      public void close() {
            if (transport_.isOpen()) {
                transport_.close();
      }
      

      I think it would be better if we simply relied on the close() method of the TIOStreamTransport, which closes the input and output streams if they have been created. Additionally, it would be better to create the input stream on the first read and the output stream on the first write: otherwise, we will get an error during closing if we're only reading (as TIOStreamTransport will still try to close the output side, resulting in writes from the deflater).

      I have a first version of a fix for these issues: I'll turn it into a pull request on Github.

        Attachments

          Activity

            People

            • Assignee:
              codesf Randy Abernethy
              Reporter:
              bluezio Antonio GarcĂ­a

              Dates

              • Created:
                Updated:
                Resolved:

                Issue deployment