mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lecharny <elecha...@gmail.com>
Subject Re: [MINA 3] Future usage
Date Mon, 01 Mar 2010 16:55:20 GMT
On 3/1/10 4:25 PM, Alan D. Cabrera wrote:
>> - it's used when trying to dispose a Service. We create a 
>> disposalFuture, instance of a ServiceOperationFuture, just to wait 
>> for all the session to be done before closing the service. It would 
>> be better to use a specific data structure, like a countdownLatch, to 
>> do that.
> Second, we should be more careful when we use Futures. In Mina 2, we 
> are misusing Futures in at least two cases :
> That seems like an implementation issue.  I think that there's merit 
> in using a well known pattern such as Future.
It all depends on what you want to do. Here, it's really a rendez-vous, 
nothing that a Future can be helpful in, except if you implement some 
more logic in it. Why using something that does not fit exatcly what we 
need, when a CountdownLatch does the job ?
>> - The ConnectFuture usage is really bothersome : we use it not only 
>> to wait for a connection to be done, but also to detect the 
>> disconnection. It's totally wrong.
>> - We also have a ReadFuture which is a bit different beast : it's 
>> only used on client side for people who want to implement a 
>> synchronous operation on top of the asynchronous operations MINA 
>> provides. We should reconsider this approach and make it separated 
>> from the main hierarchy.
>> Otherwise, we should also pick the correct Java objects to manage 
>> synchronization between the Future user (the thread waiting on the 
>> future) and the thread updating it. Right now, it's done through a 
>> setValue(blah) call, but the blah object may be almost anything, so 
>> it does not carry a lot of information.
> Can you give a more concrete example?  What  extra information does 
> one need and when?
The current implementation uses more than one condition to determinate 
if the Future is done :
- a 'ready' flag,
- a 'waiters' counter
- and an event (in our case, a call to setValue() with either an Object 
instance, a session instance of an exception instance)

The setValue(), based on the ready flag value, and on the waiters value, 
will notify all the blocked threads.

I think it's way too complicated, just because for the ConnectFuture we 
don't mange only the connection establishement, but its state since the 
initialization until the closure, so we mix many concerns.

As soon as we distinguish the connection establishement from the session 
disconnection, we are in better shape.

Also a cancellation should not be done by calling a setValue(CANCEL) nor 
should we define an established session as a ConnectFure for which the 
setValue(session) has been called. Or we should rename the setValue() 
method to setState(), to reflect what it really does.

The problem is that this setValue() method is not used in the same 
context for, say, the WriteFuture operation. Here, we just wait for the 
session.write(data) to be done, we don't really care about the fact that 
some session has been stored into the future. We should just use a 
ReentrantLock in this case, as only the thread which wrote the data is 
interested in getting the asynchronous result of a write.

They are two different beasts, they should be managed differently.
>> We should also leverage the standard API, which has a isCancelled() 
>> method, not available in Mina 2 API. Using synchronized blocks is 
>> probably not the best solution...
> Can you give a concrete example to add color to this statement?
We just have one operation that can be cancelled, the ConnectFuture. All 
other futures can't be cancelled.

(there should be a new line between those two sentences in my mail. The 
two sentences aren't connected)

We use synchronized( lock ) all over the Future code, we may benefit of 
ReentrantLock which has the big advantage of combining a locking 
mechanism, and to have a tryLock( timeout) method. Way better than 
writing wait( timeout ) methods that deal with timeout ourselves...

Emmanuel L├ęcharny

View raw message