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...
--
Regards,
Cordialement,
Emmanuel Lécharny
www.nextury.com
