Hey Ole,
> Looking into the source, I see no easy fix apart
> for a busy-wait loop (and a busy-wait loop is IMHO
> too expensive here).
> Not seeing an easy fix, and knowing that this does
> not happen with entity beans, I turn to see how this
> is done in EntityInstanceInterceptor. To my horror
> this looks like it _is_ using a busy-wait.
> IMO Simon has done a fine job on the cache and locking
> issues, but maybe there is still a to-do here?
Yes :)
I wasn't happy as well. I did quite a lot of heavy (really heavy) testing
and the busy wait loop wasn't involved almost never, so as a first step I
said "OK for now". But of course this is only an excuse for my laziness ;-),
and not the optimal solution.
> So my proposal is:
> A new method is added to the EnterpriseContext with
> this signature:
> void waitTxReentry(Transaction tx);
> This method is only used with CMT. It will wait
> until the context transaction is null or it equals
> the tx argument.
> Another new method is added to EnterpriseContext with
> signature:
> void waitNotLocked();
> A call to this method will wait until the context is
> not locked.
> For both methods it is not guaranteed that the
> condition is true on return, so rechecking like is
> currently done in EntityInstanceInterceptor is needed.
> Their implementation can be done with the good old
> wait()-notifyAll() primitives.
> In both StatefulSessionInstanceInterceptor and Entity-
> InstanceInterceptor a loop like in EntityInstance-
> Interceptor is used for retrying. The calls to the
> new EnterpriseContext methods are wrapped in a temporary
> release of the mutex, like:
> mutex.release();
> try {
> ctx.waitNotLocked();
> } finally {
> mutex.aquire();
> }
>
> This is just a very quick analysis and fix proposal.
> Does the mutex really need to be released while waiting?
Well, there are few issues here.
1) There is one busy-wait loop really "necessary", the one that involves
transactions. The other one should throw a RemoteException due to reentrancy
not allowed. It is a very old issue, never fixed.
2) In the "horrifying" tx busy-loop ;-), there is no *need* to release the
mutex, since the context cannot be passivated (it holds a tx, see
EntityInstanceCache.canPassivate), but releasing it will result in less
contention (if I release the mutex, another client with same tx can enter
and do its job). Anyway no problem to do it as Ole suggested (wrapping the
call inside a release-finally-acquire).
3) To avoid the busy-wait we'll need another call symmetric to waitTxReentry
(no need to be "only" CMT, since entity EJB are only CMT, and for session
reentrancy is not allowed), say notifyTxReentry, that must be called on
context freeing (in case of exceptions) and on transaction ended (before
afterCompletion method finishes). Anyway, I don't see why the tx argument,
Ole may you expand on this ?
4) If I have 20 clients with 20 different tx, one will run, and 19 waiting
in waitTxReentry. What happens if the one running throws an exception and
thus the context is freed ? It is not recommended IMHO to wait on the
EnterpriseContext object, but then we need another pair (id, object) - like
(id, mutex) - that has a strong identity (it is not pooled as
EnterpriseContexts are).
5) Anyway I think the exception case is not treated properly in the II,
since in case of exception the context are not freed.
Any comments ?
Simon
