[Hadoop-common-dev] [jira] Created: (HADOOP-3073) SocketOutputStream.close() should close the channel.

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12581700#action_12581700 ]

Raghu Angadi commented on HADOOP-3073:
--------------------------------------

Now I see the source of confusion. I have search shortcut for javadoc to go to Java SE 5.0. [SE 6|http://java.sun.com/javase/6/docs/api/java/net/Socket.html#getOutputStream()] explicitly states that closing this stream closes the socket, but [SE 5.0|http://java.sun.com/j2se/1.5.0/docs/api/java/net/Socket.html#getOutputStream()] doesn't.

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Robert Chansler updated HADOOP-3073:
------------------------------------

Fix Version/s: (was: 0.17.0)

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12581769#action_12581769 ]

Raghu Angadi commented on HADOOP-3073:
--------------------------------------

I think this is blocker for 0.17. The fix is also safe and simple.

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Raghu Angadi updated HADOOP-3073:
---------------------------------

Attachment: HADOOP-3073.patch

Patch attached. SocketInputStream and SocketOutputStream close the channel when close() is invoked by the user.

Also these two classes have getChannel() call so that underlying channel could be used for something thing like FileChannel.transferTo().

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12582365#action_12582365 ]

Tsz Wo (Nicholas), SZE commented on HADOOP-3073:
------------------------------------------------

Do we need try-finally in close()? i.e.
{code}
try{
reader.channel.close();
} finally {
reader.close();
}
{code}

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12582371#action_12582371 ]

Raghu Angadi commented on HADOOP-3073:
--------------------------------------

I don't think so. The fact that close() throws exception implies to user something is wrong and its ok for something to be wrong at that time.

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12582374#action_12582374 ]

Tsz Wo (Nicholas), SZE commented on HADOOP-3073:
------------------------------------------------

+1 the patch looks good

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12582373#action_12582373 ]

Raghu Angadi commented on HADOOP-3073:
--------------------------------------

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Raghu Angadi updated HADOOP-3073:
---------------------------------

Status: Patch Available (was: Open)

Thanks Nicholas. This a blocker for 0.17.

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Robert Chansler updated HADOOP-3073:
------------------------------------

Fix Version/s: 0.17.0
Priority: Blocker (was: Major)
Description:
When the stream returned by Socket.getOutputStream() is closed, it closes the socket too. So hadoop.SocketOutputStream and hadoop.SocketInputStream should do the same.



was:

When the stream returned by Socket.getOutputStream() is closed, it closes the socket too. So hadoop.SocketOutputStream and hadoop.SocketInputStream should do the same.

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12582472#action_12582472 ]

Hadoop QA commented on HADOOP-3073:
-----------------------------------

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12378602/HADOOP-3073.patch
against trunk revision 619744.

@author +1. The patch does not contain any @author tags.

tests included +1. The patch appears to include 3 new or modified tests.

javadoc -1. The javadoc tool appears to have generated 1 warning messages.

javac +1. The applied patch does not generate any new javac compiler warnings.

release audit +1. The applied patch does not generate any new release audit warnings.

findbugs -1. The patch appears to introduce 1 new Findbugs warnings.

core tests +1. The patch passed core unit tests.

contrib tests +1. The patch passed contrib unit tests.

Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2068/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2068/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2068/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2068/console

This message is automatically generated.

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12582475#action_12582475 ]

Raghu Angadi commented on HADOOP-3073:
--------------------------------------

I think this findbugs warning need to be waived. close() removed below was added to fix the same findbugs warning in HADOOP-2346 as described in a [comment|http://issues.apache.org/jira/browse/HADOOP-2346?focusedCommentId=12575116#action_12575116].

affected code: {noformat}
- try {
- //write the header.
- out.writeShort( DATA_TRANSFER_VERSION );
- out.write( OP_READ_BLOCK );
- out.writeLong( blockId );
- out.writeLong( startOffset );
- out.writeLong( len );
- out.flush();
- } finally {
- IOUtils.closeStream(out);
- }
+ //write the header.
+ out.writeShort( DATA_TRANSFER_VERSION );
+ out.write( OP_READ_BLOCK );
+ out.writeLong( blockId );
+ out.writeLong( startOffset );
+ out.writeLong( len );
+ out.flush();
{noformat}

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12582518#action_12582518 ]

Raghu Angadi commented on HADOOP-3073:
--------------------------------------

I just committed this. - Only way I can think of avoiding findbugs warning is to write a FilterSocket class.

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Raghu Angadi updated HADOOP-3073:
---------------------------------

Resolution: Fixed
Status: Resolved (was: Patch Available)

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12582617#action_12582617 ]

Hudson commented on HADOOP-3073:
--------------------------------

Integrated in Hadoop-trunk #443 (See [http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/443/])

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Raghu Angadi updated HADOOP-3073:
---------------------------------

Attachment: HADOOP-3073-javadoc.patch

I committed this with javadoc errors by mistake. will commit attached javadoc.patch to fix that.

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12582993#action_12582993 ]

Hudson commented on HADOOP-3073:
--------------------------------

Integrated in Hadoop-trunk #444 (See [http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/444/])

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Raghu Angadi updated HADOOP-3073:
---------------------------------

Release Note: SocketOutputStream.close() closes the underlying channel. Increase compatibility with java.net.Socket.getOutputStream. User Impact : none.

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Raghu Angadi updated HADOOP-3073:
---------------------------------

Description: (was: When the stream returned by Socket.getOutputStream() is closed, it closes the socket too. So hadoop.SocketOutputStream and hadoop.SocketInputStream should do the same.

)

[ https://issues.apache.org/jira/browse/HADOOP-3073?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nigel Daley updated HADOOP-3073:
--------------------------------

Component/s: ipc