1
\$\begingroup\$

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;
}
\$\endgroup\$
3
\$\begingroup\$

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
  }
\$\endgroup\$
  • 1
    \$\begingroup\$ 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. \$\endgroup\$ – Marc-Andre Oct 24 '14 at 19:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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