On Mon, Jan 28, 2013 at 11:09 AM, Emmanuel Lécharny <[email protected]>wrote:
> Le 1/28/13 10:05 AM, Jeff MAURY a écrit : > > On Mon, Jan 28, 2013 at 12:13 AM, Emmanuel Lécharny <[email protected] > >wrote: > > > >> Le 1/27/13 10:21 PM, Jeff MAURY a écrit : > >>> Hello, > >>> > >>> I started to review the Mina3 code and I have a couple of points I want > >> to > >>> share with you: > >>> > >>> 1) It is possible to close all existing connections established by a > >>> client, the method is called disconnect (BTW a log message is issued > but > >>> with an address which is the last one connected so it may be > confusing), > >>> but there is no corresponding method for the server: the unbind method > >>> simply stop listening for new connections but existing ones are kept > >> alive. > >>> I think we should align for the server but we should rename it like > >> release > >>> or shutdown but we need to think about the lifecycle of those service > >>> objects: once this method has been called, can the user call again > >> connect ? > >> This is absolutely right : we need to add a method that shutdown the > >> server as we have one for the client. > >> > >> However, those methods are semantically different : > >> - a client can close a connection at will > >> - a server must be cautious when shuting down as some client can still > >> expect some messages. > >> > > I think we should have the same requirements for client and server: when > > you stop them, you may want them so send the queued messages or not. > > There is a big difference between a client and a server : the server > serves may client when a client is contacting a few servers, on it onw > requests. > > However, it would be good to provide the same feature for both, would it > be only for the sake of simplicity : a shutdown() and a halt() methods > added in the IoService interface would be used by both the client and > the server. (I suggest shutdown and halt, but we can pick some other > names. In my mind, halt is brutal when shutdown is soft. Another > solution would be to pass an optional boolean parameter, like in > shutdown(boolean now).) > > > >> In MINA 2, we had a dispose() method which can be immediate or differed > >> (ie, the server will not accept any new request, but will cleanly wait > >> for all the current resquests to be proceeded before closing the > >> connections.) > >> > >> We should implement the same kind of behavior in a MINA 3 server. > >>> 2) I noticed that some threads are created through the SelectorPool > >>> implementations, and those threads are lost forever when the client or > >>> server is gargage collected. Is there any reasons why the > implementations > >>> are not related to the Java Executor framework. We should think of a > >>> release mechanism when the SelectorPool has been allocated by the > >> IOService. > >> Can you be a bit more explicit ? Where are thos threads created ? > >> > > When you create a NioTcpClient (see line 70), > > 1) you create a NioSelectorLoop --> 1 thread created > > 2) you create a FixedSelectorLoopPool (with 2 as second argument) --> > > creates 2 NioSelectorLoop -> creates 2 threads > You can avoid the creation of the pool, and only use the selectorLoop to > handle all the requests. That being said, if we dont destroy the created > threads when we dispose the server/client, this is badly wrong and > should be fixed. A JIRA would help here. > I will handle it as soon as next question is a next question is answered > > Btw, do you guys think that we should create a MINA3 dedicated JIRA > space, or not ? > +1 > > Thnks ! > > -- > Regards, > Cordialement, > Emmanuel Lécharny > www.iktek.com > > -- Jeff MAURY "Legacy code" often differs from its suggested alternative by actually working and scaling. - Bjarne Stroustrup http://www.jeffmaury.com http://riadiscuss.jeffmaury.com http://www.twitter.com/jeffmaury
