Using the same service & DAO from two threads

Question

Hi guys,

I've made a very simple REST controller method with Spring 3.0.0.RC1 that uses hibernate to perform a query. The query takes about ten seconds to complete. I've made this with intent so that I can fire up two requests to my controller.

Then I fire up the two requests, and query in MySQL (my DB backend) "show full processlist", and to my big surprise, there is only one request going on. One request will succeed, one request will fail with with the exception "org.hibernate.SessionException: Session is closed!" If I do more than two requests, only one will succeed, the others will fail in the same way. And there will always be just one query at a time, even though there should be multiple.

How can this be? Any suggestions?

To tell you a bit about my configuration, here is configuration that I use for the controller:

<bean id="dataSource" class="org.apache.commons.dbcp.BasicDataSource">
  <property name="driverClassName" value="com.mysql.jdbc.Driver" />
  <property name="url" value="jdbc:mysql://127.0.0.1:3306/MyDb" />
  <property name="username" value="angua" />
  <property name="password" value="vonU" />
  <property name="initialSize" value="2" />
  <property name="maxActive" value="5" />
</bean>

<bean id="sessionFactory" class="org.springframework.orm.hibernate3.annotation.AnnotationSessionFactoryBean">
  <property name="dataSource" ref="dataSource"/>
  <property name="annotatedClasses">
    <list>
      <value>tld.mydomain.sample.entities.User</value>
      <value>tld.mydomain.sample.entities.Role</value>
    </list>
  </property>
  <property name="hibernateProperties">
    <props>
      <prop key="hibernate.dialect">org.hibernate.dialect.MySQLDialect</prop>
      <prop key="hibernate.show_sql">false</prop>
    </props>
  </property>
</bean>

<bean id="transactionManager" class="org.springframework.orm.hibernate3.HibernateTransactionManager">
  <property name="sessionFactory" ref="sessionFactory"/>
</bean>

<bean class="org.springframework.dao.annotation.PersistenceExceptionTranslationPostProcessor"/>

<bean name="openSessionInViewInterceptor" class="org.springframework.orm.hibernate3.support.OpenSessionInViewInterceptor">
  <property name="sessionFactory" ref="sessionFactory"/>
  <property name="flushMode" value="0" />
</bean> 

<bean id="txProxyTemplate" class="org.springframework.transaction.interceptor.TransactionProxyFactoryBean" abstract="true">
  <property name="transactionManager" ref="transactionManager"/>
  <property name="transactionAttributes">
    <props>
      <prop key="create*">PROPAGATION_REQUIRED</prop>
      <prop key="update*">PROPAGATION_REQUIRED</prop>
      <prop key="delete*">PROPAGATION_REQUIRED</prop>
      <prop key="*">PROPAGATION_SUPPORTS,readOnly</prop>
    </props>
  </property>
</bean>

<bean id="userService" parent="txProxyTemplate">
  <property name="target">
    <bean class="tld.mydomain.business.UserServiceImpl"/>
  </property>
  <property name="proxyInterfaces" value="tld.mydomain.business.UserService"/>
</bean>

<context:component-scan base-package="tld.mydomain"/>  

<bean class="org.springframework.web.servlet.mvc.annotation.DefaultAnnotationHandlerMapping">
  <property name="interceptors">
    <list>
      <ref bean="openSessionInViewInterceptor" />
    </list>
  </property>
</bean>

<bean class="org.springframework.web.servlet.view.InternalResourceViewResolver" p:prefix="" p:suffix=".jsp"/>

<bean name="jsonView" class="org.springframework.web.servlet.view.json.JsonView">
  <property name="encoding" value="ISO-8859-1"/>
  <property name="contentType" value="application/json"/>
</bean>

and finally my controller code:

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.*;
import org.springframework.web.servlet.ModelAndView;
import org.springframework.web.servlet.view.json.JsonView;

import tld.mydomain.sample.business.UserService;

@Controller
@RequestMapping("/exp/*")
public class ExperimentsController {

 @Autowired
 private UserService userService;

 @Autowired
 private JsonView jsonView;

 @RequestMapping(value="/long", method = RequestMethod.GET)
 public ModelAndView lang() {
  ModelAndView mav = new ModelAndView(jsonView);
  userService.longQuery("UserA");
  userService.longQuery("UserB");
  return mav;
 }
}

UPDATE: Here is UserServiceImpl

public class UserServiceImpl extends AbstractCRUDServiceImpl<User, String> {

@SuppressWarnings("unchecked")
@Override
public List<User> longQuery(String username) {
  String like = "0" + username + "-%";
  return DAO.getSession().createCriteria(User.class).setResultTransformer(Criteria.DISTINCT_ROOT_ENTITY).addOrder(Order.asc("name"))
       .createCriteria("interests").add(Restrictions.like("userPrefixedId", like))
          .createCriteria("community").add(Restrictions.like("userPrefixedAuthorId", like))
          .createCriteria("member").add(Restrictions.like("userPrefixedGroupId", like))
          .add(Restrictions.isNotEmpty("skills"))
          .list();
  }
}

(The query is intentionally made slow so that I could easily reproduce the error for having multiple requests running at the same time and seeing how many simultaneous queries were running in the database)

And you'll need my AbstractCRUDServiceImpl and GenericCRUDDAO as well:

public abstract class AbstractCRUDServiceImpl<Entity extends PublishableEntity, PkID extends Serializable> implements CRUDService<Entity, PkID> {

    protected GenericCRUDDAO<Entity, PkID> DAO = new GenericCRUDDAO<Entity, PkID>(dataType());

    @Override
    public void create(Entity entity) {
     DAO.create(entity);
    }

    @Override
    public void delete(Entity entity) {
     DAO.create(entity);
    }

    @Override
    public Entity read(PkID entityPk) {
     return DAO.read(entityPk);
    }

    @Override
    public void update(Entity entity) {
     DAO.update(entity);
    }

    private Class<PkID> pkType = null;
    @SuppressWarnings("unchecked")
    public Class<PkID> pkType() {
     if(pkType != null)
      return pkType;

     // Backup solution in case datatype hasn't been set
     Type type = getClass().getGenericSuperclass();
     if (type instanceof ParameterizedType) {
      ParameterizedType paramType = (ParameterizedType) type;
      pkType = (Class<PkID>) paramType.getActualTypeArguments()[1];
     } else if (type instanceof Class) {
      pkType = (Class<PkID>) type;
     }

     return pkType;
    }

    private Class<Entity> dataType = null;
    @SuppressWarnings("unchecked")
    private Class<Entity> dataType() {
     if(dataType != null)
      return dataType;

     // Backup solution in case datatype hasn't been set
     Type type = getClass().getGenericSuperclass();
     if (type instanceof ParameterizedType) {
      ParameterizedType paramType = (ParameterizedType) type;
      dataType = (Class<Entity>) paramType.getActualTypeArguments()[0];
     } else if (type instanceof Class) {
      dataType = (Class<Entity>) type;
     }

     return dataType;
    }
}

In GenericCRUDDAO, PublishableEntity is where all my entities descend from. It has a few simple convenience-methods such as checking if the entity is valid and what parts of it should be published vs kept to itself when used in a toString or similar

public class GenericCRUDDAO<EntityType extends PublishableEntity, PkID extends Serializable> implements CRUDDAO<EntityType, PkID> {

    public GenericCRUDDAO() {}

    public GenericCRUDDAO(Class<EntityType> datatype) {
     this.setDataType(datatype);
    }

    private static SessionFactory sessionFactory = null;
    public void setSessionFactory(SessionFactory sf) {
     System.err.println("Setting SessionFactory for class " + this.getClass().getName());
     sessionFactory = sf;
    }

    private Session session = null;

    public Session getSession() {

     if(session != null) {
      if(session.isOpen())
       return session;
     }

     if(sessionFactory == null)
      Util.logError("sessionFactory is null");
     session = ((SessionFactory) sessionFactory).getCurrentSession();
     return session;
    }

    public void create(EntityType entity)
    {
     getSession().save(entity);
    }

    @SuppressWarnings("unchecked")
    public EntityType read(PkID id)
    {
     return (EntityType) getSession().get(dataType(), id);
    }

    public void update(EntityType entity)
    {
     getSession().update(entity);
    }

    public void delete(EntityType entity) {
     getSession().delete(entity);
    }

    public void delete(PkID id)
    {
     EntityType entity = read(id);
     getSession().delete(entity);
    }

    private Class<EntityType> dataType = null;
    @SuppressWarnings("unchecked")
    private Class<EntityType> dataType() {
     if(dataType != null)
      return dataType;

     // Backup solution in case datatype hasn't been set
     Type type = getClass().getGenericSuperclass();
     if (type instanceof ParameterizedType) {
      ParameterizedType paramType = (ParameterizedType) type;
      dataType = (Class<EntityType>) paramType.getActualTypeArguments()[0];
     } else if (type instanceof Class) {
      dataType = (Class<EntityType>) type;
     }

     return dataType;
    }

    public void setDataType(Class<EntityType> datatype) {
     this.dataType = datatype;
    }
}

I hope the configuration and code make it obvious why I only seem to be able to do one query at a time without them going into one-anothers feet.

Cheers

Nik

Answer 1

Looks pretty standard to me.

This of course assumes that:

• JsonView is thread safe - which I think it is
• Your implementation of UserService is also thread safe

Normally with this style of service singleton they are unless you have done something like keeping state in a member of UserServiceImpl

From what I can see in GenericCRUDDAO I'd pay close attention to the member session. If GenericCRUDDAO are singleton (one per domain object by the look of it) then you are going to run in to a bit of bother there.

The implementation of getSession() could actually be shorted to:

public Session getSession() {
    return ((SessionFactory) sessionFactory).getCurrentSession();
}

This should be thread safe, assuming that the sessionFactory is using thread local sessions.

Answer 2

After having written my update, I've been looking at the same code over and over and over again, until it hit me what I kept looking at:

 private Session session = null;
 public Session getSession() {

        if(session != null) {
                if(session.isOpen())
                        return session;
        }

        if(sessionFactory == null)
                Util.logError("sessionFactory is null");
        session = ((SessionFactory) sessionFactory).getCurrentSession();
        return session;
    }

Since the service is a singleton, and it inherits from AbstractCRUDSerivceImpl, that news a DAO, "private Session session" in effect becomes a static instance. And "if(session.isOpen()) return session;" becomes a race condition. I've reduced the function now to:

public Session getSession() {
    return ((SessionFactory) sessionFactory).getCurrentSession();
}

This seems to solve my problem. Does this look like a solution to you, or do I still have other glaring issues?

Cheers

Nik