2 Replies Latest reply: Feb 1, 2008 1:41 AM by ChaoHuang RSS

    Locker.setTxnTimeout broken?

    249828
      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