commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles <gil...@harfang.homelinux.org>
Subject Re: svn commit: r1823550 - in /commons/proper/net/trunk/src: changes/changes.xml main/java/org/apache/commons/net/telnet/TelnetClient.java
Date Thu, 08 Feb 2018 15:11:30 GMT
On Thu, 8 Feb 2018 13:26:25 +0000, sebb wrote:
> On 8 February 2018 at 12:54, Gilles <gilles@harfang.homelinux.org> 
> wrote:
>> On Thu, 8 Feb 2018 12:42:55 +0000, sebb wrote:
>>>
>>> On 8 February 2018 at 12:08, Gilles <gilles@harfang.homelinux.org> 
>>> wrote:
>>>>
>>>> Hi.
>>>>
>>>> On Thu, 08 Feb 2018 11:55:10 -0000, sebb@apache.org wrote:
>>>>>
>>>>>
>>>>> Author: sebb
>>>>> Date: Thu Feb  8 11:55:10 2018
>>>>> New Revision: 1823550
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1823550&view=rev
>>>>> Log:
>>>>> NET-643 NPE when closing telnet stream
>>>>>
>>>>> [...]
>>>>>
>>>>> Modified:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> 
>>>>> commons/proper/net/trunk/src/main/java/org/apache/commons/net/telnet/TelnetClient.java
>>>>> URL:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> 
>>>>> http://svn.apache.org/viewvc/commons/proper/net/trunk/src/main/java/org/apache/commons/net/telnet/TelnetClient.java?rev=1823550&r1=1823549&r2=1823550&view=diff
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> 
>>>>> ==============================================================================
>>>>> ---
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> 
>>>>> commons/proper/net/trunk/src/main/java/org/apache/commons/net/telnet/TelnetClient.java
>>>>> (original)
>>>>> +++
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> 
>>>>> commons/proper/net/trunk/src/main/java/org/apache/commons/net/telnet/TelnetClient.java
>>>>> Thu Feb  8 11:55:10 2018
>>>>> @@ -100,10 +100,16 @@ public class TelnetClient extends Telnet
>>>>>
>>>>>      void _flushOutputStream() throws IOException
>>>>>      {
>>>>> +        if (_output_ == null) {
>>>>> +            throw new IOException("Stream closed");
>>>>> +        }
>>>>>          _output_.flush();
>>>>>      }
>>>>>      void _closeOutputStream() throws IOException
>>>>>      {
>>>>> +        if (_output_ == null) {
>>>>> +            return;
>>>>> +        }
>>>>>          try {
>>>>>              _output_.close();
>>>>>          } finally {
>>>>
>>>>
>>>>
>>>> Why the difference in behaviour?
>>>
>>>
>>> See NET-643
>>
>>
>> I did, before writing my first reply.
>>
>>> close() is supposed to be idempotent; flush() is not supposed to be
>>> called on a closed stream.
>>
>>
>> No such requirement here:
>>  
>> https://docs.oracle.com/javase/8/docs/api/java/io/OutputStream.html#flush--
>
> I was going by the Javadoc here:
>
> 
> https://docs.oracle.com/javase/8/docs/api/java/io/OutputStreamWriter.html#close--
>
> It seems to me that ignoring flush is not the right thing to do.

The doc which you refer to reads:
---CUT---
  Closes the stream, flushing it first. Once the stream has been closed,
  further write() or flush() invocations will cause an IOException to be
  thrown. Closing a previously closed stream has no effect.
---CUT---

IIUC, the first means that, functionally, "close()" works as the
following sequence:
   out.flush();
   out.close();

However, if one does
   out.close();
   out.close();
it's fine, but
   out.flush();
   out.close();
   out.flush();
   out.close();
would throw IOException.

I don't understand how useful it is to make a difference between
two similar situations. [Calling "flush" after "close" with no
intervening "write" is asking to "transfer no data"; so no harm
done IMHO...]
Anyways, if the goal is to comply with the JDK "logic", no problem. :-)

Gilles

>>>> In the former case, "no-op" seems equally fine.
>>>
>>>
>>> It was not a no-op; it was NPE.
>>
>>
>> In the commit which I referred to, you added a "no-op" for
>> "close()" and my comment is that the same can be done for
>> "flush()" in the same circumstance (ie. no-op when "_output_"
>> is null).
>>
>>
>> Gilles


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message