Re: A CacheableSelectionKeyHandler's bug and a question about WorkerThread.detach()
Salut,
Bongjae Chang wrote:
> Hi Jeanfrancois,
>
> Perhaps, I didn't understand your words clearly about (2), so may I ask you a question?
>
> Jeanfrancois wrote:
>>> (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.
>
> I think that DefaultThreadPool is already taking charge of that.
:-) My bad.
>
> So, is the following not enough?
>
> Bongjae wrote:
>>> 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?
>
> Please advice me again.
No you are right.
Thanks!
-- Jeanfrancois
>
> --
> 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
>>
>>
>>
>>