2 Replies Latest reply on Dec 3, 2007 11:03 AM by Adrian Brock

    EJBTHREE-1142 - Incomplete fix - Generic problem

    Adrian Brock Master

      I linked this issue to where it was originally reported
      http://jira.jboss.com/jira/browse/JBAS-4980

      But the fix shows what is generally wrong with a lot of the EJB3 code
      (especially the deployment stuff).

      
       // Commit or rollback depending on the status
       if (commit == false || transaction.getStatus() == Status.STATUS_MARKED_ROLLBACK)
       {
       if (trace)
       log.trace("MessageEndpoint " + getProxyString(proxy) + " rollback");
       tm.rollback();
       }
       else
       {
       if (trace)
       log.trace("MessageEndpoint " + getProxyString(proxy) + " commit");
       tm.commit();
       }
      
       // We're done with the transaction
       transaction = null;
      


      So if tm.commit() throws an error, the transaction variable is not cleared
      meaning the problem is not fixed.
      The nulling of the transaction variable should be in the finally block further down:

       finally
       {
      + transaction = null;
      
       // Resume any suspended transaction
       if (currentTx != null)
       {
       try
       {
       tm.resume(currentTx);
       }
       catch (Throwable t)
       {
       log.warn("MessageEndpoint " + getProxyString(proxy) + " failed to resume old transaction " + currentTx);
      
       }
       }
       }
      


      Basically, the code works great under normal circumstances
      but falls apart on any error.

        • 1. Re: EJBTHREE-1142 - Incomplete fix - Generic problem
          Carlo de Wolf Master

          I was thinking about putting a TODO there.

          I don't want to disassociate when there is an error on rollback or commit. In that case we have bigger problems. The whole MessageInflowLocalProxy should be invalidated then. Hmm, except for heuristic exceptions.

          We then should resume the suspended tx. Oh bugrit, this will get complicated with the tidy up hack in there. What do we do if we have a suspended and a 'wrong' tx?

          • 2. Re: EJBTHREE-1142 - Incomplete fix - Generic problem
            Adrian Brock Master

             

            "wolfc" wrote:

            We then should resume the suspended tx. Oh bugrit, this will get complicated with the tidy up hack in there. What do we do if we have a suspended and a 'wrong' tx?


            Are you talking about
             // Suspend any bad transaction - there is bug somewhere, but we will try to tidy things up
             if (currentTx != null && currentTx.equals(transaction) == false)
             {
             log.warn("Current transaction " + currentTx + " is not the expected transaction.");
             tm.suspend();
             tm.resume(transaction);
             }
            


            The log message should really be something like:

             log.warn("SOMEBODY HAS PLAYING WITH THINGS THEY DON"T UNDERSTAND!");
            


            So there are more serious problems anyway.
            This has nothing to do with the endpoint's state so there is no need to invalidate it.
            A rollback exception when you attempt commit() is expected
            e.g. optimistic failure in the database, etc.