[ 
https://issues.apache.org/jira/browse/THRIFT-5022?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16978780#comment-16978780
 ] 

Qinghui Xu commented on THRIFT-5022:
------------------------------------

Yes, My concern is that there might be some uncaught errors (such as 
RuntimeException) that would prevent clearing `inputstream_` and 
`outputstream_`.
For me, using a flag is a cleaner way than catching all kinds of `Throwable` in 
`close`, because sometimes those throwables are expected to be caught somwhere 
higher in the call stack.

Also, with the flag approach, we don't even need to set `inputstream_` and 
`outputstream_` to `null` after close, this might be handy if some error 
happens during `close`, and a user want to inspect those underlying 
input/output streams (for example, they can try to make a heapdump of the jvm 
and check the state of those objects)

> TIOStreamTransport.isOpen is false for InputStream or Outpstream only use.
> --------------------------------------------------------------------------
>
>                 Key: THRIFT-5022
>                 URL: https://issues.apache.org/jira/browse/THRIFT-5022
>             Project: Thrift
>          Issue Type: Task
>          Components: Java - Library
>    Affects Versions: 0.13.0
>            Reporter: Andy Seaborne
>            Priority: Major
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> This follows from THRIFT-2530.
> {{TIOStreamTransaport.isOpen}} changed to be
>  
> {noformat}
>   public boolean isOpen() {
>     return inputStream_ != null && outputStream_ != null;
>   }
> {noformat}
> but constructors {{TIOStreamTransaport(InputStream)}} and 
> {{TIOStreamTransaport(OutputStream)}} leave one of {{inputStream_}} or 
> {{outputStream_}} null.
> This makes isOpen false immediately, no close() called. open() does not 
> change the state of object.
>  Example:
> {noformat}
> TIOStreamTransport x1 = new TIOStreamTransport(new ByteArrayInputStream(new 
> byte[1]));
>  System.out.println(x1.isOpen());
>  TIOStreamTransport x2 = new TIOStreamTransport(new ByteArrayOutputStream());
>  System.out.println(x2.isOpen());
> {noformat}
> is prints false both times.
>  
> It should be:
> {noformat}
>   public boolean isOpen() {
>     return inputStream_ != null || outputStream_ != null;
>   }
> {noformat}
> or an explicit flag for the open/close state but {{inputStream_ != null || 
> outputStream_ != null;}} is enough given the current close implementation.
>  
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to