Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Can this method be improved?:

public static Session getHibernateSession() throws SQLException{
    synchronized(dbSessionMutex){
        if(session == null || !session.isOpen()){
            session = HibernateUtil.getSessionFactory().openSession();
        }
        else{
            final SessionFactoryImplementor sessionFactoryImplementation = (SessionFactoryImplementor) session.getSessionFactory();
            final ConnectionProvider connectionProvider = sessionFactoryImplementation.getConnectionProvider();
            try{
                final Connection conn = connectionProvider.getConnection();
                if(conn.isClosed()){
                    session.close();
                    session = HibernateUtil.getSessionFactory().openSession();
                }
                else{
                    conn.close();
                }
            }
            catch(Exception e){
                LOGGER.error("Exception: ", e);
                session.close();
                session = HibernateUtil.getSessionFactory().openSession();
            }
        }
    }
    return session;
}
share|improve this question

1 Answer 1

No, this method cannot be improved because it is completely wrong I am afraid.

Fact: Sessions are not thread safe, don't try make them thread safe, because you don't know how to do it and there are reasons why they are not thread safe.You can simply return the Session from the factory like this

public static Session getHibernateSession() throws SQLException{
   Session session = HibernateUtil.getSessionFactory().openSession();
   return session;
}

The main problem in your code is this

 catch(Exception e){
            LOGGER.error("Exception: ", e);
            session.close();
            session = HibernateUtil.getSessionFactory().openSession();
  }

What you trying to do here? Forcing the code to get a connection?You can't do that. If you fail to open a connection first time, I doubt you will get it in 2 nanoseconds time. If you don't want to create too many instances, then you should let your IoC container manage it for you. You don't have an IoC container? Then get a Session object every time you need it and don't forget to close it when you are done

  try(Session session = getHibernateSession()){
   // use it
  }
share|improve this answer
1  
I'm not an expert with Hibernate, but I had the same feelings that getting a sessions like the OP was doing is not the way to go. –  Marc-Andre Oct 24 '14 at 19:02

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.