[
https://issues.apache.org/jira/browse/THRIFT-3392?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Antonio García updated THRIFT-3392:
-----------------------------------
Description:
While setting up a short demo project using the new Java TZlibTransport in
0.9.3, I wrote code like this:
{code:java}
try (TProtocol proto = new TJSONProtocol(new TZlibTransport(...))) {
Blog blog = new Blog();
// set up fields in blog
proto.write(blog);
}
{code}
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:
{code:java}
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);
}
{code}
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:
{code:java}
public void close() {
if (transport_.isOpen()) {
transport_.close();
}
{code}
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.
was:
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.
> Java TZlibTransport does not close its wrapper streams upon close()
> -------------------------------------------------------------------
>
> Key: THRIFT-3392
> URL: https://issues.apache.org/jira/browse/THRIFT-3392
> Project: Thrift
> Issue Type: Bug
> Components: Java - Library
> Affects Versions: 0.9.3
> Reporter: Antonio García
>
> While setting up a short demo project using the new Java TZlibTransport in
> 0.9.3, I wrote code like this:
> {code:java}
> try (TProtocol proto = new TJSONProtocol(new TZlibTransport(...))) {
> Blog blog = new Blog();
> // set up fields in blog
> proto.write(blog);
> }
> {code}
> 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:
> {code:java}
> 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);
> }
> {code}
> 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:
> {code:java}
> public void close() {
> if (transport_.isOpen()) {
> transport_.close();
> }
> {code}
> 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.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
