[jira] [Commented] (ZOOKEEPER-2369) Flushing DataOutputStream before calling toByteArray on the underlying ByteArrayOutputStream

["Patrick Hunt (JIRA)" <ji...@apache.org>]

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

Patrick Hunt commented on ZOOKEEPER-2369:
-----------------------------------------

Makes sense. I don't see why we shouldn't do what you suggest (add the flush). You see why it's a no-op currently though, right? (and why we haven't seen issues with this code)

> Flushing DataOutputStream before calling toByteArray on the underlying ByteArrayOutputStream
> --------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-2369
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2369
>             Project: ZooKeeper
>          Issue Type: Bug
>            Reporter: emopers
>            Assignee: emopers
>            Priority: Minor
>
> In ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
> {code}
>             ByteArrayOutputStream baos = new ByteArrayOutputStream();
>             BinaryOutputArchive bos = BinaryOutputArchive.getArchive(baos);
>             bos.writeInt(-1, "len");
>             rsp.serialize(bos, "connect");
>             if (!cnxn.isOldClient) {
>                 bos.writeBool(
>                         this instanceof ReadOnlyZooKeeperServer, "readOnly");
>             }
>             baos.close();
>             ByteBuffer bb = ByteBuffer.wrap(baos.toByteArray());
> {code}
> BinaryOutputArchive internally uses DataOutputStream as its stream, and when a DataOutputStream instance wraps an underlying ByteArrayOutputStream instance,
> it is recommended to flush or close the DataOutputStream before invoking the underlying instances's toByteArray() . Also, it is a good practice to call flush/close explicitly as mentioned for example http://stackoverflow.com/questions/2984538/how-to-use-bytearrayoutputstream-and-dataoutputstream-simultaneously-java.
> Moreover, "baos.close()" at second last line is not required as it is no-op according to [javadoc|http://docs.oracle.com/javase/7/docs/api/java/io/ByteArrayOutputStream.html]
> {quote}
> Closing a ByteArrayOutputStream has no effect. The methods in this class can be called after the stream has been closed without generating an IOException.
> {quote}
> The patch is to add flush method on "bos" before calling toByteArray on "baos". Similar behavior is also present in the following files:
> ./src/java/main/org/apache/zookeeper/ClientCnxn.java
> ./src/java/main/org/apache/zookeeper/server/ZKDatabase.java
> ./src/java/main/org/apache/zookeeper/server/persistence/Util.java
> ./src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java
> Let me know if this looks good. I can provide patch.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)