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
>
>
>
>