dev@grizzly.java.net

Re: About async read, write inteface and UDPConnectorHandler's blocking write

From: Bongjae Chang <carryel_at_korea.com>
Date: Sat, 23 May 2009 14:03:23 +0900

Hi,

I filed the issue #619 (https://grizzly.dev.java.net/issues/show_bug.cgi?id=619) and I attached the proposed patch.

Could you review that again in order to check in? :-)

Thanks!
--
Bongjae Chang


----- Original Message -----
From: "Jeanfrancois Arcand" <Jeanfrancois.Arcand_at_Sun.COM>
To: <dev_at_grizzly.dev.java.net>
Sent: Friday, May 22, 2009 10:44 PM
Subject: Re: About async read, write inteface and UDPConnectorHandler's blocking write


> Salut,
>
> Bongjae Chang wrote:
>> Hi,
>>
>> About (1) and (2), I filed the issue #608 and attached the proposed patch.
>>
>> About (3), I am writing a patch. But I could find that some points could be improved more.
>>
>> Now, XXXConnectorHandlers have read(...), write(...), async read and async write API separately.
>>
>> It seem that most of I/O methods are similar, so I think that it is better that common I/O operations are moved to AbstractConnectorHandler.
>>
>> Then I think that "(3) UDPConnectorHandler doesn't support a blocking write" issue could be fixed easily because write(...) API could be shared between TCPConnectorHandler and UDPConnectorHandler.
>>
>> Here is sample.
>>
>> In AbstractConnectorHandler.java
>> ---
>> public long write(ByteBuffer byteBuffer, boolean blocking) throws IOException {
>> ...
>> if(blocking) {
>> return OutputWriter.flushChannel(underlyingChannel, byteBuffer);
>> } else {
>> ...
>> try {
>> while(...) {
>> // casting not DatagramChannel in UDP or SocketChannel in TCP but WritableByteChannel in both UDP and TCP.
>> nWrite = ((WritableByteChannel)underlyingChannel).write(byteBuffer);
>> ...
>> }
>> }
>> ...
>> }
>> }
>> ---
>>
>> Of course, maybe some protocols should override the method for specific setting.
>
> Agree. But I like moving that core like you propose.
>
>>
>> And read(...) method is similar to above write(...) method.(I think that we can use ReadableByteChannel's casting).
>>
>> In addition, async read and write methods could be simplified, too. Then we can also reduce any mistakes in specific ConnectorHandler like this.
>
> Agree.
>
>>
>> In SSLConnectorHandler.java
>> ---
>> public Future<AsyncQueueWriteUnit> writeToAsyncQueue(Socket Address dstAddress, ByteBuffer buffer, AsyncWriteCallbackHandler callbackHandler, AsyncQueueDataProcessor writePreProcessor) throws IOException {
>> return writeToAsyncQueue(dstAddress, buffer, callbackHandler, writePreProcessor);
>> }
>> ---
>>
>> If user calls above method, infinite loop will be occurred because the method calls itself recursively.(Above method is current svn's code!)
>>
>> What do you think?
>
> huh! This is strange that nobody reported that issue, so nobody used
> it...I think it needs to call
>
>> /**
>> * {_at_inheritDoc}
>> */
>> public Future<AsyncQueueWriteUnit> writeToAsyncQueue(
>> SocketAddress dstAddress, ByteBuffer buffer,
>> AsyncWriteCallbackHandler callbackHandler,
>> AsyncQueueDataProcessor writePreProcessor, ByteBufferCloner cloner)
>> throws IOException {
>> isAsyncWriteQueueMode = true;
>> return selectorHandler.getAsyncQueueWriter().write(
>> underlyingChannel.keyFor(selectorHandler.getSelector()), dstAddress,
>> buffer, callbackHandler, writePreProcessor, cloner);
>> }
>
> with a null for ByteBufferCloner...Alexey?
>
> Thanks!
>
> -- Jeanfrancois
>
>>
>> Please advice me if I am misunderstandind them.
>>
>> Thanks.
>>
>> --
>> Bongjae Chang
>>
>>
>> ----- Original Message -----
>> From: "Jeanfrancois Arcand" <Jeanfrancois.Arcand_at_Sun.COM>
>> To: <dev_at_grizzly.dev.java.net>
>> Sent: Thursday, May 14, 2009 11:15 PM
>> Subject: Re: About async read, write inteface and UDPConnectorHandler's blocking write
>>
>>
>>> Salut,
>>>
>>> Bongjae Chang wrote:
>>>> Hi,
>>>>
>>>> I have several proposals and questions.
>>>>
>>>> (1)
>>>> CacheableConnector doesn't implement AsyncQueueWritable and
>>>> AsyncQueueReadable interface.
>>>>
>>>> I think that it is not difficult that CacheableConnector implements
>>>> AsyncQueueWritable and AsyncQueueReadable interfaces.
>>>>
>>>> What do you think?
>>> +1
>>>
>>>>
>>>> (2)
>>>> XXXConnectorHandler already implements AsyncQueueWritable and
>>>> AsyncQueueReadable interfaces except for CacheableConnector, so how
>>>> about moving two interfaces to ConnectorHandler interface like this?
>>>> ----
>>>> public interface ConnectorHandler extends Handler, Closeable,
>>>> *AsyncQueueWritable, AsyncQueueReadable* {
>>>> ...
>>>> }
>>>> ----
>>>>
>>>> Then, does ConnectorHandler owe too many duties?
>>> My goal when I created the ConnectorHandler interface was to let people
>>> implement their own...after more than 2 years, I've never see someone
>>> using that interface directly. This is one of the reason why such low
>>> level API got dropped in 2.0. So +1 for your proposal. I don't think it
>>> add too many duties.
>>>
>>>
>>>>
>>>> (3)
>>>> UDPConnectorHandler doesn't support a blocking write like this.
>>>> ----
>>>> public long write(ByteBuffer byteBuffer, boolean blocking) throws
>>>> IOException {
>>>> ...
>>>> if( blocking) {
>>>> *throw new IllegalStateException("Blocking mode not supported");*
>>>> } else {
>>>> ...
>>>> }
>>>> }
>>>> ----
>>>>
>>>> I am curious to know the reason.
>>> I think we just never had a chance to fully implement and test it. If
>>> you look in OutputWriter, we added support for blocking but we never
>>> finished it. So I would be interested to get you proposal on that.
>>>
>>> Thanks!!
>>>
>>> -- Jeanfrancois
>>>
>>>
>>>
>>>>
>>>> Please advice me.
>>>>
>>>> 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
>>>
>>>
>>>
>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
>
>
>
>