Hi,
"Bordet, Simone" wrote:
> > 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?
For session beans, the waiting is not spec-compliant
like Rickard pointed out. Maybe it would be an idea
to have this as an option like Dan suggested, but
with the default according to the spec. I guess such
option could also be used for entity beans.
I wonder how other container providers handle this.
According to the EJB spec, _any_ call to an entity
bean (if more than one client) could result in a
RemoteException. Client would have to catch it,
detect if it was due to an attempted reentrant
call (how?), and re-issue the invocation.
In the following I talk about entity beans only.
> 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.
You mean the re-entrant flag for entity beans that
is not yet implemented?
Yes, EJB 1.1 section 9.1.12 (and EJB 2.0 section
11.1.12) states that in this case a RemoteException
must be thrown if the bean is not reentrant. Guess
this is similar to the stateful session reentrancy
issue.
If the bean is marked reentrant, the bean provider's
programming gets so hairy that I'd prefer not to
think about it, but I guess the container just has
to pass the call to the bean, if the transaction
context is the same.
> 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).
Just wondering: We might have an active invocation
with _no_ transaction. In that case, shouldn't another
invocation to the same instance with a transaction
be delayed to avoid _any_ new invocation with a
transaction throwing RemoteException due to reentrant
call while the active call with no transaction is
active?
> 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 ?
My idea is to handle this locally in the
EnterpriseContext, waitTxReentry() would wait()
if the condition was not true, and setTransaction()
would do a notifyAll().
> 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).
Not sure I follow you here. But then, I am not sure
I really understand the locking as it is now.
With the current code, if we have one active invocation
and 19 other invocations busy-waiting with different
transaction contexts, wouldn't freeing the context
result in a NullPointerException when another thread
tries to do ctx.getTransaction()?
Don't know the cache well, but I guess that a new
context instance would be delivered from there on
the next loop.
Could we get around that case this by breaking the
wait by doing ctx.setTransaction(null) before freeing
the old context? In the next round of the loop everybody
would then be waiting in the new context.
Also, in this case would we end up using an old mutex?
> 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 ?
Not that it matters much for this discussion, but
there is something I just need to get out of my
system:
<RANT TOPIC="RemoteException">
I don't like this exception!
It is checked so it has to be declared anywhere it
might be thrown, and I don't like it. I know Suns
arguments for making this a checked exception, but
the same arguments apply to out-of-memory conditions
so why isn't OutOfMemoryError a checked exception?
This exception is simply used in too many places;
immediately comes into mind:
- Transient communication failures.
- Permanent communication failures.
- Transient server failures.
- Permanent server failures.
- EJB reentrancy failures.
- Unexpected EJB bean failures.
And what do we have to distinguish between these
cases? Sometimes we have a reference to the "real"
exception that happened, but in many cases we only
have a string that may vary between implementations.
And the EJB specifications are not very helpful in
specifying this proper: Instead of mandating that
a particular subclass be thrown they just say
"RemoteException must be thrown" in most cases.
How nice it would be if EJB specified
"ReentrancyException extends RemoteException" so
that a client could just retry a call to a busy
EJB instance. Currently EJB just says that a
RemoteException be thrown in case of illegal
reentrancy, but it is really impossible for
a client to distinguish this case from the huge
number of other cases where a RemoteException
is also thrown!
</RANT>
Best Regards,
Ole Husgaard.
