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