hmm I think the queue sync was added (by you?) because even if it's a conncurent queue, the write registration could fail if a thead write in the queue while the selector loop write it.
On Fri, Nov 9, 2012 at 11:24 PM, Emmanuel Lécharny <[email protected]>wrote: > Hi, > > I have a few comments about the existing code : > > 1) We don't need to protect the WritQueue with a lock : it's already a > thread safe queue. Removing this lock will save a lot of contention. > 2) The NioTcpSession.ready() method now calls two methods : one to > process the read, and another one to process the writes. > 3) We *must* execute the read and the writes in a separate thread, > otherwise the future will block for ever if we have more than one > message to write. In fact, theready() call should be done within the > selector worker, and delegated to a separate thread : > > Selector Thread Worker Thread > | | > +->select() | > | read() ---(1)--> processRead() > | | | > | | ... // do whatever is needed on the handler > | | | > | write() -(2)--> processWrites() > | | > +-----+ > loop > > > Here, even if the worker thread is busy processing the read, and that > includes writing some data then waiting for those data to be written, > the selector thread will continue to process incoming data and outgoing > data. The communication between the Selector thread and teh Worker > thread is done through a queue of events and a notification mechanism > using the Futures. > > One more thing : we should use the same Worker Thread for a given > session, otherwise we may have some problems while decoding the > messages. If the user wants to be able to process two different messages > concurrently for the same session, then it's up to this user to add an > executor in the handler. > > -- > Regards, > Cordialement, > Emmanuel Lécharny > www.iktek.com > >
