Hey Marc,
I looked over the new Entity Interceptors and locking and have a few
comments
- There is a race condition when an Entity is re-entrant. In
EntityInstanceInterceptor.invoke you set the context's transaction in the
synchronized(ctx) block rather than in the synchronized(ctx.getTxLock())
block. Thus it is possible for 2 different threads in 2 different
transactions to get through the first Tx synchronized block and both do
ctx.setTransaction(...). I suggest maybe that you put the synchronized(ctx)
block inside the synchronized(ctx.getTxLock()) block. I hope I'm making
sense here.
- In EntitytInstanceInterceptor.invoke in the finally block. If an
exception is thrown, you should do a ctx.getTxLock().notifyAll() as well as
ctx.notifyAll(). An exception could happen in loadEntity, thus, no Sync
would be registered with the TM, thus threads waiting on ctx.getTxLock()
would never be awakened.
- I really think that doing a wait() rather than a wait(5000) is a really
really bad idea. If a transaction timeout happens, I don't think a thread
that is in a ctx.wait() or a ctx.getTxLock().wait() will ever wakeup and
actually do a rollback and release all it's locked entities and resources.
I am VERY unfamiliar with the TM code, but take a look at
TxCapsule.timedOut(). This method only marks the transaction as ROLLING
back and does nothing else. I can find no where in the jboss codebase where
Thread.interrupt() is called in relation to transaction timeouts. (I'd test
out this theory myself, but I'm on "vacation" and won't really be able to
until Monday.)
I guess there is 2 solutions here. One, have transaction timeouts interrupt
their threads, or Two, have a wait(5000) block in EntityInstanceInterceptor
as well as:
// Maybe my transaction already expired?
if (mi.getTransaction() != null && mi.getTransaction().getStatus() ==
Status.STATUS_MARKED_ROLLBACK)
throw new RuntimeException("Transaction marked for rollback, possibly a
timeout");
at the beginning of your while(ctx==null) loop. Actually, it would probably
be best to have a wait(Tx.timeout) instead of just 5000.
Anyways, FREEZING, is very very bad in itself. Yeah sure, JBoss might not
have any deadlocks, but you really want to be able to recover from
Application deadlock.
- I'll probably open up a different email thread on this, but maybe all the
locks should be pulled out of EnterpriseContext and be managed by some
LockManager. If multiple instances for option B and C is ever
implemented(I'd like to tackle this), they'll have to use a different
locking mechanism because you wouldn't be able to synchronize on ctx and
ctx.getTxLock().
Well, I hope I'm making sense here. It's the 4th and I have a belly full of
beer and BBQ.
Regards,
Bill
_______________________________________________
Jboss-development mailing list
[EMAIL PROTECTED]
http://lists.sourceforge.net/lists/listinfo/jboss-development
