Hi,
I created the issue #597 and attached the proposed patch to which I applied your opinion.
https://grizzly.dev.java.net/issues/show_bug.cgi?id=597
And I hope that you will modify it if it still needs to be reformed.
Thanks.
--
Bongjae Chang
----- Original Message -----
From: "Jeanfrancois Arcand" <Jeanfrancois.Arcand_at_Sun.COM>
To: <dev_at_grizzly.dev.java.net>
Sent: Tuesday, May 12, 2009 11:37 PM
Subject: Re: OutputWriter's flushChannel() issue
> 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
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
>
>
>
>