Potential Bug in EntityManagerSessionImpl

[schtho]

Hello everyone,

I was writing a small benchmark testing a quite simple process that manipulates a JPA Entity by calling Services (CDI-Beans). As soon as I tried to run several instances  one after another, the system stopped responding due to a lack of available database connections.

We are currently using Activiti 5.20.0, Hibernate 5.1.1, Weld 2.2.11 on Tomcat 7.0.47. The ProcessEngine is configured based upon a CdiStandaloneProcessEngineConfiguration, including

          conf.setJpaHandleTransaction(true);
          conf.setJpaCloseEntityManager(true);
‍‍‍‍

Debugging through the stack lead me to

org.activiti.engine.impl.variable.EntityManagerSessionImpl.close()‍

.

I'm a little bit puzzled about the check within this method:

    if (closeEntityManager && entityManager != null && !entityManager.isOpen()) {
      try {
        entityManager.close();
      } catch (IllegalStateException ise) {
        throw new ActivitiException("Error while closing EntityManager, may have already been closed or it is container-managed", ise);
      }
    } 
‍‍‍‍‍‍‍‍‍

Is this intended? Checking for the entityManager to be not open prior to closing it? Shouldn’t it check for the entityManager to be open?

    if (closeEntityManager && entityManager != null && entityManager.isOpen()) {
‍‍‍

I’ve fetched the 5.20.0 sources, modified the check and built it. The unit tests ran fine and my benchmark executed as expected.

I can’t imagine being the first person to stumble over this, it feels like I am wrong in my presumptions.

Best regards,

Thomas

[vasile_dirla]

Hi Thomas,

Yes, you are right, it should be closed if it is opened. I didn't test it  it by myself yet but according to the code I think it's a bug.
Please add also some tests for your change and then creating a PR for this change will give you all the credit for this fix.
Thanks for your input.