Salut,
Bongjae Chang wrote:
> Hi,
>
> When I sent many large packets to the remote server with
> OutputWriter#flushChannel(), some packets could be lost with receiving
> the following exception.
>
> In the sender side,
> ----
> java.io.IOException: Client disconnected
> at
> com.sun.grizzly.util.OutputWriter.flushChannel(OutputWriter.java:124)
> at
> GrizzlyTCPConnectorWrapper.send(GrizzlyTCPConnectorWrapper.java:61)
> at
> GrizzlyTCPConnectorWrapper.send(GrizzlyTCPConnectorWrapper.java:44)
> at GrizzlyNetworkManager.send(GrizzlyNetworkManager.java:218)
> at
> GrizzlyNetworkManagerSendTest$2.run(GrizzlyNetworkManagerSendTest.java:78)
> at java.lang.Thread.run(Thread.java:717)
> ----
>
> First, I thought that the server might close the client's connection
> because of overflow, and the exception log said "Client disconnected",
> so I tuned server's params.
>
> But this error could not be resolved unfortunately.
>
> When I tried to debug the server and client, I was very confused because
> the connection was still alive and had no problems.
>
> I used the OutputWrite for sending packets like this.
>
> ----
> *long timeout = 30000; // 30sec*
> ...
> OutputWriter.flushChannel( connectorHandler.getUnderlyingChannel(),
> message, *timeout*);
> ...
> ----
>
> Intuitively, I thought that flushChannel() would retry and wait for at
> least the timeout if the server and client were busy.
>
> But I saw the code, I could know that my thought was wrong.
>
> Here is OutputWriter#flushChannel(SelectableChannel channel, ByteBuffer
> bb, long writeTimeout)' code.
> ----
> public static long flushChannel(SelectableChannel channel, ByteBuffer
> bb, long writeTimeout) throw IOExceiton{
> ...
> try{
> WritableByteChannel writableChannel = (WritableByteChannel) channel;
> while( bb.hasRemaining() ) {
> len = writableChannel.write(bb);
> if( len>0 ){
> attempts = 0;
> nWrite += len;
> } else {
> attempts++;
> ...
> * if(writeSelector.select(writeTimeout) == 0) { ------ (1)*
> if(attempts > 2)
> * throw new IOException("Client disconnected");
> ------ (2)*
> }
> }
> }
> } catch( IOException ex ) {
> ...
> } finally {
> ...
> }
> return nWrite;
> }
> ----
>
> (1) When I tested, Selector#select() could return 0 though it was not
> timed out. Then OutputWriter#flushChannel()'s writeTimeout is perhaps
> meaningless. Actually above the wrong IOException will be thrown before
> OutputWriter#flushChannel() waits for writeTimeout and tries to send the
> message again. Of course, attempts has 2 limit value, but I think that
> it is not enough.
>
> (2) I think that "Client disconnected"'s message is wrong. If client
> disconnected, the following exception will usually be thrown rather than
> above (2).
> ----
> java.nio.channels.ClosedChannelException
> at
> sun.nio.ch.SocketChannelImpl.ensureWriteOpen(SocketChannelImpl.java:236)
> at sun.nio.ch.SocketChannelImpl.write(SocketChannelImpl.java:434)
> at
> com.sun.grizzly.util.OutputWriter.flushChannel(OutputWriter.java:106)
> at
> GrizzlyTCPConnectorWrapper.send(GrizzlyTCPConnectorWrapper.java:61)
> at
> GrizzlyTCPConnectorWrapper.send(GrizzlyTCPConnectorWrapper.java:44)
> at GrizzlyNetworkManager.send(GrizzlyNetworkManager.java:218)
> at
> GrizzlyNetworkManagerSendTest$2.run(GrizzlyNetworkManagerSendTest.java:78)
> at java.lang.Thread.run(Thread.java:717)
> ----
> I think that "Client is busy or timed out"'s message is better.
Agree.
>
> So, I tried to make the patch again experimentally.
>
> ----
> public static long flushChannel(SelectableChannel channel, ByteBuffer
> bb, long writeTimeout) throw IOExceiton{
> ...
> * long elapsedTime = 0;*
> try{
> WritableByteChannel writableChannel = (WritableByteChannel) channel;
> while( bb.hasRemaining() ) {
> len = writableChannel.write(bb);
> if( len>0 ){
> attempts = 0;
> nWrite += len;
> } else {
> attempts++;
> ...
> //if(writeSelector.select(writeTimeout) == 0) { ------ (1)
> // if(attempts > 2)
> // throw new IOException("Client disconnected");
> ------ (2)
> //}
> *long startTime = System.currentTimeMillis();*
> * int selected = writeSelector.select(writeTimeout);*
> * elapsedTime += (System.currentTimeMillis() - startTime);*
> * if( selected == 0 ) {*
> * if(attempts > 2 && (writeTimeout > 0 && elapsedTime >=
> writeTimeout ) )*
> * throw new IOException("Client is busy or timed out");*
> * }*
> }
> }
> } catch( IOException ex ) {
> ...
> } finally {
> ...
> }
> return nWrite;
> }
> ----
>
> Then flushChannel() will try to write the buffer again until at least
> the writeTimeout.(I don't know that I should reset the elapsedTime when
> the buffer is written successfully)
>
> And I could know that all packets were received successfully without any
> errors after I applied my proposed patch.
I agree with the patch, but I think I would move the elapsedTime
calculation inside the if (selected == 0) to minimize that calculation.
Also I would think elapsedTime not be cumulative, e.g. as soon as we
have a successful write, we reset the value.
What do you think? Just file an issue and attach you patch and I will do
the appropriate test and apply.
Thanks!
-- Jeanfrancois
>
> Thanks.
>
> --
> Bongjae Chang
>
>