[jira] [Commented] (DIRMINA-982) ProtocolEncoderOutputImpl.flush() throws an IllegalArgumentException if buffers queue is empty

["Jenya Pisarenko (JIRA)" <ji...@apache.org>]

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

Jenya Pisarenko commented on DIRMINA-982:
-----------------------------------------

I'm looking at the 2.0 source. The {{ProtocolEncoderOutputImpl.flush()}} here looks as follows:
{code:java}
        public WriteFuture flush() {
            Queue<Object> bufferQueue = getMessageQueue();
            WriteFuture future = null;

            while (!bufferQueue.isEmpty()) {
                Object encodedMessage = bufferQueue.poll();

                if (encodedMessage == null) {
                    break;
                }

                // Flush only when the buffer has remaining.
                if (!(encodedMessage instanceof IoBuffer) || ((IoBuffer) encodedMessage).hasRemaining()) {
                    future = new DefaultWriteFuture(session);
                    nextFilter.filterWrite(session, new EncodedWriteRequest(encodedMessage, future, destination));
                }
            }

            if (future == null) {
                // Creates an empty writeRequest containing the destination
                WriteRequest writeRequest = new DefaultWriteRequest(null, null, destination);
                future = DefaultWriteFuture.newNotWrittenFuture(session, new NothingWrittenException(writeRequest));
            }

            return future;
        }
{code}
Queue iteration part is ok. As future variable is assigned only in a loop, The last if consequent is invoked, when the loop condition is never true within the method, i.e. {{bufferQueue.isEmpty() == true}} by the {{flush()}} call. It should be possible to achieve in two sequential {{flush()}} calls without any concurrency (assuming the {{getMessageQueue()}} can return an empty collection).

The first line of the if block:
{code:java}
                // Creates an empty writeRequest containing the destination
                WriteRequest writeRequest = new DefaultWriteRequest(null, null, destination);
{code}
always fails, because of the {{IllegalArgumentException}}, thus the rest of it:
{code:java}
                future = DefaultWriteFuture.newNotWrittenFuture(session, new NothingWrittenException(writeRequest));
{code}
is never executed.

I suggest to fix the last part of the method so it could correctly create an empty write request/write future.

> ProtocolEncoderOutputImpl.flush() throws an IllegalArgumentException if buffers queue is empty
> ----------------------------------------------------------------------------------------------
>
>                 Key: DIRMINA-982
>                 URL: https://issues.apache.org/jira/browse/DIRMINA-982
>             Project: MINA
>          Issue Type: Bug
>    Affects Versions: 2.0.7
>            Reporter: Jenya Pisarenko
>
> https://git-wip-us.apache.org/repos/asf?p=mina.git;a=blob;f=mina-core/src/main/java/org/apache/mina/filter/codec/ProtocolCodecFilter.java;h=f413491e058ff1c5a24c3b2ccae6d736f481bbaf;hb=2.0#l428
> First, flush method initializes WriteFuture with a null reference.
> The object can be constructed later, if buffers queue isn't empty, but otherwise remains untouched. 
> If it still holds a null reference, the DefaultWriteRequest constructor call always fails, as it doesn't accept null as a message param.
> Excerpt from the stack trace:
> {noformat}
> Caused by: java.lang.IllegalArgumentException: message
> at org.apache.mina.core.write.DefaultWriteRequest.<init>(DefaultWriteRequest.java:133)
> at org.apache.mina.filter.codec.ProtocolCodecFilter$ProtocolEncoderOutputImpl.flush(ProtocolCodecFilter.java:448)
> at com.ugcs.messaging.mina.MinaEncoder.encode(MinaEncoder.java:27)
> at org.apache.mina.filter.codec.ProtocolCodecFilter.filterWrite(ProtocolCodecFilter.java:308)
> ... 33 more
> {noformat}



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