This is a fix for QPID-2432

Modified the XAResourceImpl to maintain the timeout value and set it to any XID in the start method if the time value > 0
Also the XID nulled after commit, rollback and forget, to prevent a timeout being set on an invalid xid.
The setTimeout method will only set the timeout if xid is not null and if the timeout value is different from the previous value.

Modified the test cases in FaulTest to adhere to the correct behaviour and also added a new test case to cover the issue mentioned in the JIRA.


git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@919666 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 77 insertions, 37 deletions

diff --git a/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java b/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java
index 35adda9348..4f4fc3ddd3 100644
--- a/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java
+++ b/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java
@@ -48,8 +48,13 @@ public class XAResourceImpl implements XAResource
      */
     private Xid _xid;
 
+    /**
+     * The time for this resource
+     */
+    private int _timeout;
+    
     //--- constructor
-
+   
     /**
      * Create an XAResource associated with a XASession
      *
@@ -90,6 +95,10 @@ public class XAResourceImpl implements XAResource
             _xaSession.createSession();
             convertExecutionErrorToXAErr(e.getException().getErrorCode());
         }
+        finally
+        {
+            _xid = null;
+        }
         checkStatus(result.getStatus());
     }
 
@@ -171,6 +180,10 @@ public class XAResourceImpl implements XAResource
             _xaSession.createSession();
             convertExecutionErrorToXAErr(e.getException().getErrorCode());
         }
+        finally
+        {
+            _xid = null;
+        }
     }
 
 
@@ -178,30 +191,13 @@ public class XAResourceImpl implements XAResource
      * Obtains the current transaction timeout value set for this XAResource instance.
      * If XAResource.setTransactionTimeout was not used prior to invoking this method,
      * the return value is the default timeout i.e. 0;
-     * otherwise, the value used in the previous setTransactionTimeout call is returned.
      *
      * @return The transaction timeout value in seconds.
      * @throws XAException An error has occurred. Possible exception values are XAER_RMERR, XAER_RMFAIL.
      */
     public int getTransactionTimeout() throws XAException
     {
-        int result = 0;
-        if (_xid != null)
-        {
-            Future<GetTimeoutResult> future =
-                    _xaSession.getQpidSession().dtxGetTimeout(convertXid(_xid));
-            try
-            {
-                result = (int) future.get().getTimeout();
-            }
-            catch (SessionException e)
-            {
-                // we need to restore the qpid session that has been closed
-                _xaSession.createSession();
-                convertExecutionErrorToXAErr(e.getException().getErrorCode());
-            }
-        }
-        return result;
+        return _timeout;
     }
 
     /**
@@ -325,6 +321,10 @@ public class XAResourceImpl implements XAResource
             _xaSession.createSession();
             convertExecutionErrorToXAErr( e.getException().getErrorCode());
         }
+        finally
+        {
+            _xid = null;
+        }
         checkStatus(result.getStatus());
     }
 
@@ -340,25 +340,29 @@ public class XAResourceImpl implements XAResource
      */
     public boolean setTransactionTimeout(int timeout) throws XAException
     {
-        boolean result = false;
-        if (_xid != null)
+        _timeout = timeout;
+        if (timeout != _timeout && _xid != null)
+        {            
+            setDtxTimeout(_timeout);
+        }
+        return true;
+    }
+    
+    private void setDtxTimeout(int timeout) throws XAException
+    {
+        try
         {
-            try
-            {
-                _xaSession.getQpidSession()
-                        .dtxSetTimeout(XidImpl.convert(_xid), timeout);
-            }
-            catch (QpidException e)
+            _xaSession.getQpidSession()
+                    .dtxSetTimeout(XidImpl.convert(_xid), timeout);
+        }
+        catch (QpidException e)
+        {
+            if (_logger.isDebugEnabled())
             {
-                if (_logger.isDebugEnabled())
-                {
-                    _logger.debug("Cannot convert Xid into String format ", e);
-                }
-                throw new XAException(XAException.XAER_PROTO);
+                _logger.debug("Cannot convert Xid into String format ", e);
             }
-            result = true;
+            throw new XAException(XAException.XAER_PROTO);
         }
-        return result;
     }
 
     /**
@@ -413,6 +417,10 @@ public class XAResourceImpl implements XAResource
         }
         checkStatus(result.getStatus());
         _xid = xid;
+        if (_timeout > 0)
+        {
+            setDtxTimeout(_timeout);
+        }
     }
 
     //------------------------------------------------------------------------
@@ -477,7 +485,15 @@ public class XAResourceImpl implements XAResource
                 throw new XAException(XAException.XAER_DUPID);
             case NOT_FOUND:
                 // The XID is not valid.
-                throw new XAException(XAException.XAER_NOTA);
+                try
+                {
+                   throw new XAException(XAException.XAER_NOTA);
+                }
+                catch (XAException e)
+                {
+                    e.printStackTrace();
+                    throw e;
+                }
             case ILLEGAL_STATE:
                 // Routine was invoked in an inproper context.
                 throw new XAException(XAException.XAER_PROTO);
diff --git a/java/systests/src/main/java/org/apache/qpid/test/unit/xa/FaultTest.java b/java/systests/src/main/java/org/apache/qpid/test/unit/xa/FaultTest.java
index 1e5932b6db..47705f8105 100644
--- a/java/systests/src/main/java/org/apache/qpid/test/unit/xa/FaultTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/test/unit/xa/FaultTest.java
@@ -339,7 +339,7 @@ public class FaultTest extends AbstractXATestCase
         {
             assertEquals("Wrong error code: ", XAException.XAER_PROTO, e.errorCode);
         }
-    }
+    }    
 
     /**
      * Strategy:
@@ -355,7 +355,7 @@ public class FaultTest extends AbstractXATestCase
         _xaResource.end(xid, XAResource.TMSUCCESS);
         xid = getNewXid();
         _xaResource.start(xid, XAResource.TMNOFLAGS);
-        assertEquals("Wrong timeout", _xaResource.getTransactionTimeout(), 0);
+        assertEquals("Wrong timeout", _xaResource.getTransactionTimeout(), 1000);
     }
 
     /**
@@ -381,5 +381,29 @@ public class FaultTest extends AbstractXATestCase
             assertEquals("Wrong error code: ", XAException.XA_RBTIMEOUT, e.errorCode);
         }
     }
+    
+    /**
+     * Strategy:
+     * Set the transaction timeout to 1000
+     */
+    public void testTransactionTimeoutAfterCommit() throws Exception
+    {
+        Xid xid = getNewXid();
+        
+        _xaResource.start(xid, XAResource.TMNOFLAGS);
+        _xaResource.setTransactionTimeout(1000);
+        assertEquals("Wrong timeout", 1000,_xaResource.getTransactionTimeout());
+        
+        //_xaResource.prepare(xid);
+        _xaResource.end(xid, XAResource.TMSUCCESS);
+        _xaResource.commit(xid, true);
+        
+        _xaResource.setTransactionTimeout(2000);
+        assertEquals("Wrong timeout", 2000,_xaResource.getTransactionTimeout());
+        
+        xid = getNewXid();
+        _xaResource.start(xid, XAResource.TMNOFLAGS);
+        assertEquals("Wrong timeout", 2000, _xaResource.getTransactionTimeout());
+    }
 
 }