I have two things for the patch:

1) There is minor mistake that ThreadLocal variable is sometimes not correctly cleared in PicketLinkIDMOrganizationServiceImpl.endRequest(). There is code snipet like:

public void endRequest(ExoContainer container)
   {
      Boolean b = txStarted.get();
      if (b == null || !b)
      {
         // The tx has not already been started or startRequest has not been called so we can leave the method
         return;
      }      
      try
      {
         ....
      }
      finally
      {
         // Clear the ThreadLocal to prevent memory leak
         txStarted.remove();
      }
   }

In case that "b" is false, the method is immediately returned and it never reach the try/catch/finally block, which means that ThreadLocal is not cleared in this case.

2) Second thing is, that patch is not always optimal solution for all cases. Sometimes there is still start of Hibernate transaction even if it's not needed. Thing is that Picketlink IDM itself has it's own caching layer. It uses JBoss cache and results of IDM operations are cached here.
So overally the patch is good for HTTP request for anonymous user because there is not any call to OrganizationService (and Picketlink IDM) operations. But for HTTP request of logged user, Hibernate transaction is still always started even if it's sometimes not needed. Like this scenario:

  • Logged user sends request to http://localhost:8080/portal/classic/
  • Method UserProfileDAOImpl.findUserProfileByName("mary") is called
  • This method invokes Picketlink IDM operation for finding user, so it needs to call PicketlinkIDMService.getIdentitySession() and it starts Hibernate transaction
  • Picketlink IDM operations are called, but all results of these operations are already cached in Picketlink IDM cache from previous calls.
  • PicketlinkIDMOrganizationService.endRequest() finish Hibernate transaction, but transaction was not needed for this HTTP request as there were calls to Picketlink IDM, but not calls to Hibernate.

So at this moment, we will probably wait with the patch for aplying into GateIn trunk as we want to provide some solution in Picketlink IDM level to start transaction only when it's really needed (aka - items are not found in PL IDM cache). JIRA is already opened https://issues.jboss.org/browse/PLIDM-32.

Another potential solution is to use JTA, which is described here https://community.jboss.org/wiki/JTAIntegrationWithGateIn . In JTA environment, Hibernate automatically start transaction only when it really needs it, so this may be good for your purpose.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira