Excellent! frankly I will try to read tonight, finishing up the stateful stuff (minor rewrite). read real fast: the wait() as opposed to wait(5000), I know :) I do it on purpose, if there is a lock I WANT TO KNOW ABOUT IT. In fact wait(5000) only makes things cosmetically better, a missed notify is bad and we would need to see it. That's all. marcf |-----Original Message----- |From: [EMAIL PROTECTED] |[mailto:[EMAIL PROTECTED]]On Behalf Of Bill |Burke |Sent: Wednesday, July 04, 2001 4:34 PM |To: Jboss-Development@Lists. Sourceforge. Net |Subject: [JBoss-dev] comments on new instance interceptors and locking | | |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 _______________________________________________ Jboss-development mailing list [EMAIL PROTECTED] http://lists.sourceforge.net/lists/listinfo/jboss-development
