OFBiz
  1. OFBiz
  2. OFBIZ-1755

Odd bug related to begin()/commit() transaction

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Invalid
    • Affects Version/s: Trunk
    • Fix Version/s: None
    • Component/s: ALL COMPONENTS
    • Labels:
      None

      Description

      This bug is difficult to explain. Worse, it is not 100% reproducable. If you run the enclosed testCommit() service right after starting up Ofbiz, it happens every time. However, subsequent invocations of testCommit() may or may not cause the problem.

      The attached code demonstrates the problem completely. Just unzip in in your hot-deploy folder, run ant, restart ofbiz and then using the service manager, run "testCommit". The attached view is based on ItemIssuance, so you must have shipped an item or two. However, I expect that the code could be modified to use Party or whatever.

      To fully understand this bug, you must modify EntityListIterator.next() by adding e.printStackTrace() right below:

      } catch (SQLException e) {

      Otherwise, the real problem is masked by the following attempt at close();

      Basically, If you have an EntityListIterator. created as shown, then you subsequently do a TransactionUtil.begin() and TransactionUtil.commit(), the next EntityListIterator.next() call results in the exception:
      java.sql.SQLException: ResultSet not open. Operation 'next' not permitted. Verify that autocommit is OFF.
      at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(Unknown Source)
      ...
      at org.apache.commons.dbcp.DelegatingResultSet.next(DelegatingResultSet.java:169)
      ...
      at org.ofbiz.test.Tests.testCommit(Tests.java:91)

      This bug did not exist in the MinervaConnectionFactory code. I have only just seen it because I was trying to move my code from a December 15th release to the latest.

      I originally thought this only existed in Derby. However, the same error happens with Postgres.

      If you modify the test code by putting "isActiveTransaction" inside the commit, it runs just fine. This is of course because there is already an active transaction and no commit() is really done.

      What is attempted here is a common thing whereby you read data, do some computations, then write data in between begin() and commit().

      I may be doing it wrong, but the fact remains that this worked for me for months until this latest download (which is probably long after the change was made).

      1. testcommit.zip
        7 kB
        Skip Dever
      2. Tests.java
        5 kB
        Skip Dever

        Activity

        Hide
        Skip Dever added a comment - - edited

        David

        Well, you are right in one regard. Using suspend()/resume() works to clear up the problem.
        I have attached a modified Tests.java file that uses it and runs fine in all cases.

        However, you are WAY wrong in another. This is NOT simple JTA stuff. If you use the original Tests.java file and simply rename the service in the xml file to say ATestCommit, then run it, it runs just fine. This is because testCommit is an existing Ofbiz service and has a seca on it.

        This current code is very fragile in this regard. Adding a seca should not cause the underlying service to fail.

        Additionally, I attempted to use the suspend()/resume() technique on my real code and although the EntityListIterator.next() worked just fine, the store operation failed with GenericEntityException:
        (A lock could not be obtained within the time requested)

        Finally, I copied this begin - getListIterator - commit method from old Ofbiz code. I have seen it used in half a dozen places. It all used to work just fine.

        In my view, it is a bug when modifications break existing code.

        I was tempted to go back to the old code base because it looks more stable. However, the use of the new geronimo code base is appealing enough to spend the time required to make it all work.

        Show
        Skip Dever added a comment - - edited David Well, you are right in one regard. Using suspend()/resume() works to clear up the problem. I have attached a modified Tests.java file that uses it and runs fine in all cases. However, you are WAY wrong in another. This is NOT simple JTA stuff. If you use the original Tests.java file and simply rename the service in the xml file to say ATestCommit, then run it, it runs just fine. This is because testCommit is an existing Ofbiz service and has a seca on it. This current code is very fragile in this regard. Adding a seca should not cause the underlying service to fail. Additionally, I attempted to use the suspend()/resume() technique on my real code and although the EntityListIterator.next() worked just fine, the store operation failed with GenericEntityException: (A lock could not be obtained within the time requested) Finally, I copied this begin - getListIterator - commit method from old Ofbiz code. I have seen it used in half a dozen places. It all used to work just fine. In my view, it is a bug when modifications break existing code. I was tempted to go back to the old code base because it looks more stable. However, the use of the new geronimo code base is appealing enough to spend the time required to make it all work.
        Hide
        David E. Jones added a comment -

        The error mentioned is saying that you cannot call next() on a ResultSet that is closed.

        When you commit or rollback a transaction it closes the connections related to it, which will also close ResultSets, and will make them unusable.

        It's not a bug, it's how it is supposed to work.

        Yes, it is possible to write code that will result in error messages.

        If you want operations in transactions independent of the main transaction you need to use the suspend/resume features (part of JTA, and on the TransactionUtil class in OFBiz), and this is easiest by calling a service with the require-new-transaction attribute set to true.

        But seriously, this is fairly basic JTA stuff and if this doesn't make sense start by learning about that.

        Show
        David E. Jones added a comment - The error mentioned is saying that you cannot call next() on a ResultSet that is closed. When you commit or rollback a transaction it closes the connections related to it, which will also close ResultSets, and will make them unusable. It's not a bug, it's how it is supposed to work. Yes, it is possible to write code that will result in error messages. If you want operations in transactions independent of the main transaction you need to use the suspend/resume features (part of JTA, and on the TransactionUtil class in OFBiz), and this is easiest by calling a service with the require-new-transaction attribute set to true. But seriously, this is fairly basic JTA stuff and if this doesn't make sense start by learning about that.

          People

          • Assignee:
            David E. Jones
            Reporter:
            Skip Dever
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development