OK so this is new. I pulled the source. The tracking of the filter position on the IoSession is the problem. I don't remember the old style filter chain having that problem. However the old style wasn't as efficient. If the write chain did occur in the selector loop then it wouldn't be a problem. Either that or redesign the filter chain flow to fix this problem. On Aug 16, 2014 3:24 PM, "Kai ZHANG" <[email protected]> wrote:
> But before the message added into writeQueue, it will be pass down through > the filter chain. > The filter operation is runned in the calling thread. > > If two thread call write() on the same time, then the target IoSession's > filter chain will be iterated in two threads simultaneously. But the > IoSession has a 'writeChainPosition' variable to store the offset of next > filter to run, which may be modified by two threads without any > synchronization mechanism. > > On 08/17/2014 02:59 AM, Jon V. wrote: > >> It is my understanding that the write is occurring on the the selector >> loop. This occurs on the same selector loop every time and therefore is >> thread-safe. When you call write the params are encapsulated and triggered >> for execution on the select loop. It does not occur in the same stack as >> the caller of write() >> On Aug 16, 2014 2:45 PM, "Jens Reimann" <[email protected]> wrote: >> >> Yes, but the call to write() can originate from any thread and thus is >>> not >>> thread safe. All handlers in the write call get called unsynchronized. >>> This >>> also causes issues at other locations like the gzip filter. >>> >>> There are two options, synchronize write calls yourself by surrounding >>> calls to write() with a mutex lock or semaphore. >>> >>> Or, if you cannot synchronize all write calls (e.g. when you use the >>> Heartbeat filter, which calls write on its own) you need to use the >>> ExecutorFilter (I think that was the name). This can be used to force >>> read >>> and write calls to a specific thread. >>> >>> Jens >>> >>> On Aug 16, 2014 8:18 PM, Emmanuel Lecharny <[email protected]> wrote: >>> >>>> From the top of my head (I'm in a bus atm), each session has its own >>>> >>> chain >>> >>>> of filter, thus its own instance. The variable is not shared, and the >>>> session is always handled by the same IoPricessor, so the same thread. >>>> >>> That >>> >>>> should be thread safe unless you start doing weird things with an >>>> >>> executor. >>> >>>> Le 16 août 2014 18:06, "Kai ZHANG" <[email protected]> a écrit : >>>> >>>> Hi, >>>>> >>>>> I am a beginer of Mina. I read the api doc located at: >>>>> >>>>> /http://mina.apache.org/mina-project/apidocs/org/apache/ >>>>> mina/core/session/IoSession.html/ >>>>> >>>>> It says that IoSession is thread-safe. >>>>> >>>>> But when I read the source of mina trunk branch. I found the >>>>> IoSession.write() method may be not thread-safe. The java source file >>>>> >>>> is : >>> >>>> /./core/src/main/java/org/apache/mina/session/AbstractIoSession.java/ >>>>> >>>>> Here is the method calling chain: >>>>> AbstractIoSession.write() >>>>> -> AbstractIoSession.doWriteWithFuture >>>>> -> AbstractIoSession.processMessageWriting() >>>>> -> AbstractIoFilter.messageWriting() >>>>> -> AbstractIoSession.callWriteNextFilter() >>>>> >>>>> The code which is thread-safe reside in >>>>> >>>> AbstractIoSession.callWriteNextFilter(), >>> >>>> here is the code: >>>>> >>>>> /** >>>>> * process session message received event using the filter >>>>> chain. To be called by the session {@link SelectorLoop} . >>>>> * >>>>> * @param message the received message >>>>> */ >>>>> @Override >>>>> public void callWriteNextFilter(WriteRequest message) { >>>>> if (IS_DEBUG) { >>>>> LOG.debug("calling next filter for writing for message >>>>> '{}' position : {}", message, writeChainPosition); >>>>> } >>>>> >>>>> /writeChainPosition--;/ >>>>> >>>>> if (writeChainPosition < 0 || chain.length == 0) { >>>>> // end of chain processing >>>>> enqueueWriteRequest(message); >>>>> } else { >>>>> chain[writeChainPosition].messageWriting(this, >>>>> message, >>>>> this); >>>>> } >>>>> >>>>> /writeChainPosition++;/ >>>>> } >>>>> >>>>> Here the variable "writeChainPosition" is not thread-safe, If more than >>>>> one thread call IoSession.write() concurrently, the >>>>> >>>> "writeChainPosition" >>> >>>> may have race condition. >>>>> >>>>> The result is some of the IoFilter may be skipped or called twice, and >>>>> >>>> the >>> >>>> message data passed down the filter chain may be broken. >>>>> >>>>> Could you tell me if my understanding is correct? >>>>> >>>>> Is IoSession.write() method designed to be thread-safe or should I use >>>>> >>>> a >>> >>>> lock for every concurrent IoSession.write() operation? >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >
