This discussion is archived
2 Replies Latest reply: Jan 31, 2008 11:41 PM by ChaoHuang RSS

Locker.setTxnTimeout broken?

249828 Newbie
Currently Being Moderated
We've discovered a problem with the setTxnTimeout/isTimedOut methods on the transaction objects. The problem is in the Locker object, we've written a test case to illustrate the scenario:
    @Test
    public void testSetTransaction() throws Exception {

     //Setup.  Important note is to have some timeout value set on the environment

        EnvironmentConfig config = new EnvironmentConfig();
        config.setAllowCreate(true);
        config.setTransactional(true);
        config.setTxnTimeout(500000000);

        File home = new File("test/DB");
        home.mkdirs();
       
        EnvironmentImpl env = new EnvironmentImpl(home, config);

     //create our Locker object and set the transaction timeout to 0 (0 should mean no timeout per berkeley API docs)
        Locker locker = new BasicLocker(env);     
        locker.setTxnTimeout(0);

     //wait for a short period
        Thread.sleep(100);

     //we should never be timed out, since we set the timeout to zero
        assertFalse(locker.isTimedOut());

    }
Here are the relative code snippets from Locker that cause this situation:

1) In the constructor txnStartMillis is only set if the timeout is non-0
        txnTimeOutMillis = envImpl.getTxnTimeout();

        if (txnTimeOutMillis != 0) {
            txnStartMillis = System.currentTimeMillis();
        } else {
            txnStartMillis = 0;
        }
2) When we call setTxnTimeout, the txnStartMillis is set, even if timeOut is set to 0
    /**
     * Set the timeout period for this transaction, in milliseconds.
     */
    public synchronized void setTxnTimeout(long timeOutMillis) {
        txnTimeOutMillis = timeOutMillis;
        txnStartMillis = System.currentTimeMillis();
    }
3) the isTimedOut method checks for a non-zero txnStartMillis, instead of using the txnTimeOutMillis. Since the setter put a value in txnStartMillis, it thinks it needs to check for a timeout. The diff will always be greater than 0, so this always returns true in this scenario.
    boolean isTimedOut()
        throws DatabaseException {

        if (txnStartMillis != 0) {
            long diff = System.currentTimeMillis() - txnStartMillis;
            if (diff > txnTimeOutMillis) {
                return true;
            }
        }
        return false;
    }
FIX:
We think just changing line 518 in Locker from this:
  if (txnStartMillis != 0) {
to this:
  if (txnTimeOutMillis != 0) {
Would be an appropriate fix. Simple enough that we didn't create a patch file for it, but if you want we can throw one together. Also, the setTxnTimeout(..) should maybe check for a non-zero value before setting txnStartMillis.


Let me know if you have any questions.

Donnie

edited for code formatting