[jboss-jira] [JBoss JIRA] Commented: (JBAS-5564) Probable logical problem in NonReentrantLock

[Domagoj Cosic (JIRA)]

    [ http://jira.jboss.com/jira/browse/JBAS-5564?page=comments#action_12414969 ] 
            
Domagoj Cosic commented on JBAS-5564:
-------------------------------------

I tracked down my problem now. The beforementioned logical expression in NonReentrantLock may be O.K., it does not bother me anyway. My problem is:

miTx is null although I have a transaction. 

miTx is not mi transaction at all! Is it intended? In EntityReentranceInterceptor, you read the transaction from EntityEnterpriseContext, not from MethodInvocation, hence the name is misleading. Transaction in EntityEnterpriseContext is normaly set in EntityInstanceInterceptor (to the value from MethodInvocation), but not for read only methods. I have a read only method in this case! Is it correct that the transaction is null in that case?

To conclude: If two threads access a bean in read only manner simultaneosly (which they are allowed to), the second one lands in wait with no necessity. If I make my bean non re-entrant, such a problem does not occur. It cannot be that the logic is different for re-entrant an non re-entrant beans.

> Probable logical problem in NonReentrantLock
> --------------------------------------------
>
>                 Key: JBAS-5564
>                 URL: http://jira.jboss.com/jira/browse/JBAS-5564
>             Project: JBoss Application Server
>          Issue Type: Bug
>      Security Level: Public(Everyone can see) 
>          Components: EJB2
>    Affects Versions: JBossAS-5.0.0.Beta4, JBossAS-4.2.2.GA
>         Environment: None, source code analysis
>            Reporter: Domagoj Cosic
>
> Introduction: My direct problem is distantly related to JBAS-4177. The difference is, I use an own implementation of a BeanLock, which is so far correct an not an issue in this ticket, nor is the my original problem that the NonReentrantLock causes. However, my original problem led me to NonReentrantLock and I analyzed its source code in some depth. I have a suspicion that there is a serious bug in it.
> The problem I want to point at is in the line 143:
> Up to the trunk revision there stands:
> 	if(lockHolder != curThread && (miTx == null || miTx.equals(holdingTx)))
> There is some contradiction to the description:
> "It will throw a ReentranceException if the same thread tries to acquire twice
> or the same transaction tries to acquire twice"
> For a non re-entrant bean it means, in pseudo code:
> if (sameThread or sameTransaction) {
>  throw new ReentrantException();
> } else {
>  waitForOtherThreadOrTransaction;
>  on timeout return false;
>  return true;
> }
> This description does not apply to an re-entrant bean, but for re-entrant bean it basically means (in pseudo code):
> if (sameThread or sameTransaction) {
>  return true;
> } else {
>  waitForOtherThreadOrTransaction;
>  on timeout return false;
>  return true;
> }
> I backtracked the code to the previous change, which was revision 16662 dated 27.08.2003
> There it stands:
>             if (lockHolder == Thread.currentThread()) throw new ReentranceException();
>             if (miTx != null && miTx.equals(holdingTx)) throw new ReentranceException();
> Although this code was for the non re-eantrant case, for the re-entrant case the code should return true for those cases, where ReentrantException was thrown.
> Let us transform this code step by step:
> Step 1:
>             if (lockHolder == Thread.currentThread()) return true;
>             if (miTx != null && miTx.equals(holdingTx)) return true;
> Step 2:
>             if (lockHolder == Thread.currentThread() || miTx != null && miTx.equals(holdingTx)) {
>             	return true;
>             } else {
>             	wait;
>             	return true;
>             }
> Step 3 (negating):
>             if (lockHolder != Thread.currentThread() && (miTx == null || !miTx.equals(holdingTx))) {
>             	wait;
>             	return true;
>             } else {
>             	return true;
>             }
> You see the difference? Current code lacks "!" before miTx.equals(holdingTx). It seems wrong to me and it causes a problem for me, so it would be nice if someone else takes a look at this.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira