Sorry , me former message looked strange, here it is again:
Hi Trustin,
Where in the code you see that "ExecutorFilter ensures that two threads
don't handle the same session".
You have said that before and I didn't understand it.
Looking at the ExecutorFilter(version 1.0.1) fireEvent method :
private void fireEvent( ... )
{
Event event = new Event( type, nextFilter, data );
SessionBuffer buf = SessionBuffer.getSessionBuffer( session );
synchronized( buf.eventQueue )
{
buf.eventQueue.add( event );
if( buf.processingCompleted )
{
buf.processingCompleted = false;
if ( logger.isDebugEnabled() ) {
logger.debug( "Launching thread for " +
session.getRemoteAddress() );
}
executor.execute( new ProcessEventsRunnable( buf ) );
}
}
}
There is a synch part but it synchronizes the executor.execute method
(which just adds the ProcessEventsRunnable to its workQueue)
and not the execution of the ProcessEventsRunnable itself.
Am I missing something ?
Haviv wrote:
>
>
> Hi Trustin,
>
> Where in the code you see that "ExecutorFilter ensures that two threads
> don't handle the same session".
> You have said that before and I didn't understand it.
> Looking at the ExecutorFilter(version 1.0.1) fireEvent method :
> private void fireEvent(...)
> {
> Event event = new Event( type, nextFilter, data );
> SessionBuffer buf = SessionBuffer.getSessionBuffer( session );
>
> synchronized( buf.eventQueue )
> {
> buf.eventQueue.add( event );
> if( buf.processingCompleted )
> {
> buf.processingCompleted = false;
> if ( logger.isDebugEnabled() ) {
> logger.debug( "Launching thread for " +
> session.getRemoteAddress() );
> }
>
> executor.execute( new ProcessEventsRunnable( buf ) );
> }
> }
> }
>
> There is a synch part but it synchronizes the executor.execute
> method(which just adds the ProcessEventsRunnable to its workQueue) and not
> the execution of the ProcessEventsRunnable itself.
> Am I missing something ?
>
>
>
> Trustin Lee wrote:
>>
>> Hi Maarten,
>>
>> On 3/29/07, Maarten Bosteels <[EMAIL PROTECTED]> wrote:
>>> On 3/28/07, Maarten Bosteels <[EMAIL PROTECTED]> wrote:
>>> > Trustin,
>>> >
>>> > I think there is theoretically still a chance that two threads
>>> > synchronize on different lock object for the same session:
>>> >
>>> > + private Object getDecoderLock( IoSession session )
>>> > + {
>>> > + Object lock = session.getAttribute( DECODER_LOCK );
>>> > + if( lock == null )
>>> > + {
>>> > + lock = new Object();
>>> > + session.setAttribute( DECODER_LOCK, lock );
>>> > + }
>>> > +
>>> > + return lock;
>>> > + }
>>> >
>>> > thread A calls session.getAttribute( DECODER_LOCK ) and gets null
>>> > thread B calls session.getAttribute( DECODER_LOCK ) and gets null
>>> > thread A creates a new lock object lockA and stores it in the session
>>> > thread B creates a new lock object lockB and stores it in the session
>>> > thread A locks on lockA and enters decoder.decode
>>> > thread B locks on lockB and also enters decoder.decode
>>> >
>>> > I guess you can't get around locking on the IoSession instance itself
>>> ?
>>
>> ExecutorFilter ensures that two threads don't handle the same session.
>> So I think the scenario you gave won't happen. But, will we still
>> have a visibility problem with getDecoderLock() method in this case?
>> I thought both synchronized HashMap (for 1.0) and ConcurrentHashMap
>> (for 1.1+) takes care of the visibility problem. WDYT?
>>
>> Trustin
>> --
>> what we call human nature is actually human habit
>> --
>> http://gleamynode.net/
>> --
>> PGP Key ID: 0x0255ECA6
>>
>>
>
>
--
View this message in context:
http://www.nabble.com/CumulativeProtocolDecoder-BUG---tf3466819.html#a9728362
Sent from the mina dev mailing list archive at Nabble.com.
