title: Is this a right pattern for Hibernate session/transaction management?
body: <p>we're developing an application using JSF 2.0 (primefaces) and Hibernate 3.6.1
  We're following the approch of letting the higher level of the application be agnostic about the DAL framework.... that means that we don't use the session-per-request approach, but we configured Hibernate to handle the sessione on a per-thread basis.
  We've implemented a class whose role is to handle an "atomic operation", taking care of open the session and the transaction when they don't exixt already, and to simply hook other db operation to the existing transaction, to make them part of the main operation, witount letting them know if they're executing independently or as part of a bigger operation.
  This is the code for our OperationManager:</p>
  
  <pre><code>public class OperationManager implements IOperationManager {
  
      Transaction tx = null;
      boolean isInternalTransaction = false;
  
      /* (non-Javadoc)
       * @see alekso.npe.dal.IOperationManager#beginOperation()
       */
      @Override
      public Session beginOperation(){            
          Session session = SessionFactoryUtil.getInstance().getCurrentSession();
          if (session.getTransaction().isActive()) {
              isInternalTransaction = false;  
              tx = session.getTransaction();
          }
          else {
              isInternalTransaction = true;
              tx = session.beginTransaction();
          }
          return session;
      }
  
      /* (non-Javadoc)
       * @see alekso.npe.dal.IOperationManager#commitOperation()
       */
      @Override
      public void commitOperation(){
          if (isInternalTransaction)
              tx.commit();
      }
  
      /* (non-Javadoc)
       * @see alekso.npe.dal.IOperationManager#rollbackOperation()
       */
      @Override
      public void rollbackOperation(){
          if (isInternalTransaction)
              tx.rollback();
      }
  }
  </code></pre>
  
  <p>The SessionFactoryUtil class, is the "classic" factory util for hibernate, suggested everywhere... we picked up this implementation:</p>
  
  <pre><code>public class SessionFactoryUtil {
  
  final static Logger log = Logger.getLogger(SessionFactoryUtil.class);
  
  /** The single instance of hibernate SessionFactory */
  private static org.hibernate.SessionFactory sessionFactory;
  
  /**
   * disable contructor to guaranty a single instance
   */
  private SessionFactoryUtil() {
  }
  
  static {
      // Annotation and XML
      // sessionFactory = new
      // AnnotationConfiguration().configure().buildSessionFactory();
      // XML only
      try {
          sessionFactory = new Configuration().configure().buildSessionFactory();
      } catch (Exception e) {
          log.error("Errore nella creazione del session factory", e);
          System.out.println("Errore nella creazione del session factory");
      }
  
  }
  
  public static SessionFactory getInstance() {
      try{
          return sessionFactory;
      } catch (Exception ex)
      {
          log.error("errore nella creazione della session di hibernate", ex);
          return null;
      }
  }
  
  /**
   * Opens a session and will not bind it to a session context
   * 
   * @return the session
   */
  public Session openSession() {
      return sessionFactory.openSession();
  }
  
  /**
   * Returns a session from the session context. If there is no session in the
   * context it opens a session, stores it in the context and returns it. This
   * factory is intended to be used with a hibernate.cfg.xml including the
   * following property &lt;property
   * name="current_session_context_class"&gt;thread&lt;/property&gt; This would return
   * the current open session or if this does not exist, will create a new
   * session
   * 
   * @return the session
   */
  public Session getCurrentSession() {
      return sessionFactory.getCurrentSession();
  }
  
  /**
   * closes the session factory
   */
  public static void close() {
      if (sessionFactory != null)
          sessionFactory.close();
      sessionFactory = null;
  
  }
  }
  </code></pre>
  
  <p>Now, how do we use the operation manager?
  Simply, whoever is intended to be responsable for an atomic operation on the db, call the method on OperationManager.</p>
  
  <pre><code>public class BL1{
      public void highMethod() {      
          IOperationManager om = new OperationManager();
          BL2 bl2 = new BL2();
          BL3 bl3 = new BL3();
          try {           
              om.beginOperation();            
  
              bl2.midMethod();
              bl3.midMethod();
  
              om.commitOperation();           
          } catch (Exception ex) {
              omrollbackOperation();      
          }       
      }
  }
  
  public class BL2{
      public void midMethod() {       
          IOperationManager om = new OperationManager();
          DAL1 dal1 = new DAL1();
          DAL2 dal2 = new DAL2();
          try {           
              om.beginOperation();            
  
              dal1.lowMethod1();
              dal2.lowMethod1();
  
              om.commitOperation();           
          } catch (Exception ex) {
              omrollbackOperation();      
          }       
      }
  }
  
  public class BL3{
      public void midMethod() {       
          IOperationManager om = new OperationManager();
          DAL1 dal1 = new DAL1();
          DAL2 dal2 = new DAL2();
          try {           
              om.beginOperation();            
  
              dal1.lowMethod2();
              dal2.lowMethod2();
  
              om.commitOperation();           
          } catch (Exception ex) {
              omrollbackOperation();      
          }       
      }
  }
  
  public class DAL1{
      public void lowMethod1() {      
          IOperationManager om = new OperationManager();
          Session session = nullM
          try {                                   
              session = om.beginOperation();
  
              // do some work on session
              session.saveOrUpdate(...);
              session.load(..);
              session.somethingElse(....);
  
              om.commitOperation();           
          } catch (Exception ex) {
              omrollbackOperation();      
          }       
      }
      public void lowMethod2() {      
          IOperationManager om = new OperationManager();
          Session session = nullM
          try {                                   
              session = om.beginOperation();
  
              // do some work on session
              session.saveOrUpdate(...);
              session.load(..);
              session.somethingElse(....);
  
              om.commitOperation();           
          } catch (Exception ex) {
              omrollbackOperation();      
          }       
      }
  }
  
  public class DAL2{
      public void lowMethod1() {      
          IOperationManager om = new OperationManager();
          Session session = nullM
          try {                                   
              session = om.beginOperation();
  
              // do some work on session
              session.saveOrUpdate(...);
              session.load(..);
              session.somethingElse(....);
  
              om.commitOperation();           
          } catch (Exception ex) {
              omrollbackOperation();      
          }       
      }
      public void lowMethod2() {      
          IOperationManager om = new OperationManager();
          Session session = nullM
          try {                                   
              session = om.beginOperation();
  
              // do some work on session
              session.saveOrUpdate(...);
              session.load(..);
              session.somethingElse(....);
  
              om.commitOperation();           
          } catch (Exception ex) {
              omrollbackOperation();      
          }       
      }
  }    
  }
  </code></pre>
  
  <p>Doing so, if I call the BL1.highMethod, everyting below (al the method colled from inside) will be under the transiction it started: the om.beginOperation() on the method called, simply returns the session started by BL1.highMethod, and the om.commitOperation or om.rollBackOperation of the method called, simply do nothing at all, leaving at the om instance created in BL1.highMethod the responsability to commit/rollback and close the session.
  But if we call directly BL2.midMethod, the responsability for managing session and transaction, will be its own. The same happen if we call directly DAL1.lowMethod1.
  The pattern is: if I (as a method) didn't called beginOperation, everything I use to get my job done, will handle the session/transaction on its own; if I beginOperation, it'll be my responsability to manage session/transiction.
  I hope I made it clear.</p>
  
  <p>Now, it seemed to me to be a very smart approach... until something goes wrong.
  What is going wrong, it's really hard to tell.
  Simply the application start to act weird: sometimes the application create a lock on the db, during an update (the database talk about a locking row, but looking at the sequence of sql statement flushed to the db, it has no sense); sometimes there's no lock, and no error logged, but the application seems to read "old data"... like if the operation we did couple of seconds ago, is not there... and it appear to be there just after 30 seconds (or 10, or 60... it depends). I mean, very strage behaviour. We only see some error on the log, whne we hit the rowlock on the db, that brings to a long live transaction, otherwise nothing seems wrong. To make things more complicated, the same exact sequence of user operation on the application (on the same record, byy the same user, etc...) sometimes work, sometimes not.
  And this behaviour happen from when we refactor the code introducing the pattern I showed above, using the OperationManager class.</p>
  
  <p>Anyone out there has an idea of what we did wrong, why this pattern is not working as desired?
  I apologies for the lenght of the question posted...</p>