I filed two separated issues and attached diffs.
https://grizzly.dev.java.net/issues/show_bug.cgi?id=604
https://grizzly.dev.java.net/issues/show_bug.cgi?id=605
Thanks.
--
Bongjae Chang
----- Original Message -----
From: "Jeanfrancois Arcand" <Jeanfrancois.Arcand_at_Sun.COM>
To: <dev_at_grizzly.dev.java.net>
Sent: Saturday, May 16, 2009 12:09 AM
Subject: Re: A CacheableSelectionKeyHandler's bug and a question about WorkerThread.detach()
> Salut,
>
> Bongjae Chang wrote:
>> Hi,
>>
>> I wrote a unit test case.
>>
>> Actually I just modified ProtocolParserTest because I think that it is enough in order to reproduce this issue.
>>
>> I attached the diffs of the unit test and the proposed patch.
>>
>> Here is the diff
>>
>> ----
>> Index: test/java/com/sun/grizzly/ProtocolParserTest.java
>> ===================================================================
>> --- test/java/com/sun/grizzly/ProtocolParserTest.java (revision 3193)
>> +++ test/java/com/sun/grizzly/ProtocolParserTest.java (working copy)
>> @@ -44,6 +44,7 @@
>> import com.sun.grizzly.utils.ControllerUtils;
>> import com.sun.grizzly.utils.NonBlockingTCPIOClient;
>> import com.sun.grizzly.utils.NonBlockingSSLIOClient;
>> +import com.sun.grizzly.connectioncache.server.CacheableSelectionKeyHandler;
>>
>> import java.io.File;
>> import java.io.IOException;
>> @@ -109,6 +110,33 @@
>> }
>> }
>>
>> + public void testSimplePacketUsingServerCache() throws IOException {
>> + Controller controller = createController(PORT, new CacheableSelectionKeyHandler( 10, 1 ) );
>> + NonBlockingTCPIOClient client = new NonBlockingTCPIOClient("localhost", PORT);
>> +
>> + try {
>> + byte[] testData = "Hello".getBytes();
>> + ControllerUtils.startController(controller);
>> + client.connect();
>> + client.send(testData);
>> +
>> + System.out.println("Sleeping 5 seconds");
>> + try{
>> + Thread.sleep(5 * 1000); //Wait 5 second before sending the bytes
>> + } catch (Throwable t){
>> +
>> + }
>> + client.send(" Partial".getBytes());
>> +
>> + byte[] response = new byte["Hello Partial".length()];
>> + client.receive(response);
>> + assertTrue(Arrays.equals("Hello Partial".getBytes(), response));
>> + } finally {
>> + controller.stop();
>> + client.close();
>> + }
>> + }
>> +
>> public void testSimpleSSLPacket() throws IOException {
>> Controller controller = createSSLController(PORT);
>> NonBlockingSSLIOClient client = new NonBlockingSSLIOClient("localhost", PORT,sslConfig);
>> @@ -146,12 +174,18 @@
>> }
>> }
>> }
>> -
>> +
>> private Controller createController(int port) {
>> + return createController( port, null );
>> + }
>> +
>> + private Controller createController(int port, SelectionKeyHandler selectionKeyHandler ) {
>> final ProtocolFilter echoFilter = new EchoFilter();
>> final ProtocolFilter parserProtocolFilter = createParserProtocolFilter();
>> TCPSelectorHandler selectorHandler = new TCPSelectorHandler();
>> selectorHandler.setPort(port);
>> + if( selectionKeyHandler != null )
>> + selectorHandler.setSelectionKeyHandler( selectionKeyHandler );
>>
>> final Controller controller = new Controller();
>>
>> Index: main/java/com/sun/grizzly/connectioncache/server/CacheableSelectionKeyHandler.java
>> ===================================================================
>> --- main/java/com/sun/grizzly/connectioncache/server/CacheableSelectionKeyHandler.java (revision 3193)
>> +++ main/java/com/sun/grizzly/connectioncache/server/CacheableSelectionKeyHandler.java (working copy)
>> @@ -85,6 +85,7 @@
>>
>> @Override
>> public void postProcess(SelectionKey key) {
>> + super.postProcess( key );
>> SelectableChannel channel = key.channel();
>> inboundConnectionCache.requestProcessed(channel, 1);
>> inboundConnectionCache.responseSent(channel);
>> ----
>>
>> I think that you can reproduce this issue with modified ProtocolParserTest easily.
>>
>> And if this issue will be comfirmed, I will file this issue.
>
> I was able to reproduce the issue. Go ahead and file an issue with the diff.
>
> Thanks!
>
> -- Jeanfrancois
>
>
>>
>> Thanks.
>> --
>> Bongjae Chang
>>
>>
>> ----- Original Message -----
>> From: "Bongjae Chang" <carryel_at_korea.com>
>> To: <dev_at_grizzly.dev.java.net>
>> Sent: Friday, May 15, 2009 10:34 AM
>> Subject: Re: A CacheableSelectionKeyHandler's bug and a question about WorkerThread.detach()
>>
>>
>>> Hi,
>>>
>>> Jeanfrancois Arcand wrote:
>>>> Hum...do you think you can write a unit test case that demonstrate that
>>>> issue? That will be quite usefull to add to the builld (and the fix :-))
>>> At first, I thought that you just had omitted the line "super.postProcess( key )" in CacheableSelectionKeyHandler#postProcess() by mistake.
>>>
>>> But maybe it seems that you have purposed it when I read your mail.
>>>
>>> Then, I think that a unit test case and more logs which demonstrate my words needs, too.
>>>
>>> I will try to write that and reply again.
>>>
>>> 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:51 PM
>>> Subject: Re: A CacheableSelectionKeyHandler's bug and a question about WorkerThread.detach()
>>>
>>>
>>>> Salut,
>>>>
>>>> Bongjae Chang wrote:
>>>>> I would like to illustrate some more in detail.
>>>>>
>>>>> (1)
>>>>> /Bongjae wrote:/
>>>>> />I know that ParserProtocolFilter saves the current thread's ByteBuffer
>>>>> and attaches the ThreadAttachment to SelectionKey if
>>>>> ProtocolParser's isExpectingMoreData() returns true. If detach() is not
>>>>> executed at this case, *the remaining ByteBuffer can cause a parsing
>>>>> error*./
>>>>>
>>>>> If BaseSelectionKeyHandler#postProcess() is not called like current
>>>>> CacheableSelectionKeyHandler#postProcess(), in other words
>>>>> WokerThread#detach() is not called,
>>>>>
>>>>> Current thread's byteBuffer(A) which have been stored in
>>>>> ThreadAttachment(B) will be clear() by WorkerThread#reset() because
>>>>> DefaultThreadPool#afterExecute() will be executed after executing the task.
>>>>>
>>>>> Then, the byteBuffer(A) will be shared by ThreadAttachmemt(B)'s
>>>>> byteBuffer because it is the same reference. So, the next step for
>>>>> parsing the incomplete message will use the missing data of the buffer
>>>>> because ByteBuffer's position already has been reset.
>>>> Hum...do you think you can write a unit test case that demonstrate that
>>>> issue? That will be quite usefull to add to the builld (and the fix :-))
>>>>
>>>>
>>>>>
>>>>>
>>>>> (2)
>>>>> /Bongjae wrote:/
>>>>> />When I reviewed the WorkerThread's detach() method, a question arised.
>>>>> Why does WorkerThread create new ByteBuffer in BYTE_BUFFER mode? *The
>>>>> new ByteBuffer could become to be meaningless* if the SelectionKey had a
>>>>> saved ByteBuffer, doesn't it?/
>>>>>
>>>>> If it is, I think that "byteBuffer = null" is better than
>>>>> "createByteBuffer(true)" in point of being able to cut down on the
>>>>> overhead of redundant ByteBuffer's allocation.
>>>> Agree. But we must make sure next time the WorkerThread is used, the
>>>> ByteBuffer gets properly created.
>>>>
>>>> Thanks!
>>>>
>>>> -- Jeanfrancois
>>>>
>>>>>
>>>>> Thanks.
>>>>>
>>>>> --
>>>>> Bongjae Chang
>>>>>
>>>>>
>>>>>
>>>>> ----- Original Message -----
>>>>> *From:* Bongjae Chang <mailto:carryel_at_korea.com>
>>>>> *To:* dev_at_grizzly.dev.java.net <mailto:dev_at_grizzly.dev.java.net>
>>>>> *Sent:* Wednesday, May 13, 2009 10:07 PM
>>>>> *Subject:* A CacheableSelectionKeyHandler's bug and a question about
>>>>> WorkerThread.detach()
>>>>>
>>>>> Hi,
>>>>>
>>>>> I tried to use CacheableSelectionKeyHandler, but some problems were
>>>>> occurred when ProtocolParser's isExpectingMoreData() returned true.
>>>>>
>>>>> In conclusion, CacheableSelectionKeyHandler#postProcess(SelectionKey
>>>>> key) didn't call super.postProcess( key ).
>>>>>
>>>>> In CacheableSelectionKeyHandler.java
>>>>> ----
>>>>> public void postProcess(SelectioKey key) {
>>>>> * super.postProcess( key );*
>>>>> ...
>>>>> }
>>>>> ----
>>>>>
>>>>> So WokerThread's detach() couldn't be called and the byteBuffer was
>>>>> recycled inadequately.
>>>>>
>>>>> I know that ParserProtocolFilter saves the current thread's
>>>>> ByteBuffer and attaches the ThreadAttachment to SelectionKey if
>>>>> ProtocolParser's isExpectingMoreData() returns true. If detach() is
>>>>> not executed at this case, the remaining ByteBuffer can cause a
>>>>> parsing error.
>>>>>
>>>>> When I reviewed the WorkerThread's detach() method, a question
>>>>> arised. Why does WorkerThread create new ByteBuffer in BYTE_BUFFER
>>>>> mode? The new ByteBuffer could become to be meaningless if the
>>>>> SelectionKey had a saved ByteBuffer, doesn't it?
>>>>>
>>>>> In WorkerThreadImpl.java
>>>>> ----
>>>>> public ThreadAttachment detach() {
>>>>> ...
>>>>> if((mode & Mode.BYTE_BUFFER) != 0 ) {
>>>>> createByteBuffer(true);
>>>>> }
>>>>> }
>>>>> ----
>>>>>
>>>>> And DefaultThreadPool already creates a ByteBuffer when it is equal
>>>>> to null in beforeExecute().
>>>>>
>>>>> In DefaultThreadPool.java
>>>>> ----
>>>>> protected void beforeExecute(Thread t, Runnable r ) {
>>>>> ((WorkerThreadImpl) t).createByteBuffer(false);
>>>>> }
>>>>> ----
>>>>>
>>>>> Perhaps, is it for the purpose of eliminating the dependency of the
>>>>> thread pool's impl?
>>>>>
>>>>> If detach() is modified like this,
>>>>> ----
>>>>> public ThreadAttachment detach() {
>>>>> ...
>>>>> if((mode & Mode.BYTE_BUFFER) != 0 ) {
>>>>> byteBuffer = null;
>>>>> //createByteBuffer(true);
>>>>> }
>>>>> }
>>>>> ----
>>>>>
>>>>> Could any side-effect be occurred?
>>>>>
>>>>> 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
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
>
>
>
>