(First of all, this is not my code. Secondly, to protect the guilty, I have changed the class names to hide anything specific to my employer, so if things don't correspond with each other that's why!).
Can someone help me clear up a doubt please? I have been asked to investigate a problem with a full connection pool, and this code seems to smell.
We have lots of org.hibernate.SessionException: Session is closed! in our logs, around the following code, which uses the connection pool that is behaving badly.
The calls come in via a web service:
1 @Stateless
2 @WebService(name="MyWebService", targetNamespace="http://www.mycompany.com/2010/01/WebService/myWebService")
3 @WebContext(contextRoot="/myContextRoot", secureWSDLAccess=false)
4 public class MyWebService implements IMyWebService {
6 @WebMethod
7 @NotNull
8 public ArrayList<SearchResult> performSearch(ArrayList<String> criteria) {
9 GetAllResponses caller = new GetAllResponses();
10 return caller.doSearch(criteria);
11 }
13 }
The GetAllResponses class is as follows:
1 public class GetAllResponses {
2 private static MyHome myHome = new MyHome();
4 public SearchResult doSearch(ArrayList<String> criteria) {
5 return doSearchInternal(criteria.elementAt(0), criteria.elementAt(1));
6 }
8 private SearchResult doSearchInternal(String a, String b) {
9 DataObject info = myHome.findDataObject(a, b);
10 return info.getAsSearchResult();
11 }
12 }
And MyHome is as follows:
1 @Stateless
2 @Local({MyHomeInterface.class})
3 public class MyHome {
4 private Session session;
6 public DataObject findDataObject(String a, String b) {
7 try {
8 this.session = MyHibernateUtil.getSessionFactory().getCurrentSession();
9 Transaction tx = this.session.beginTransaction();
10 //do stuff with session and return a DataObject
11 } catch (Exception ex) {
12 ex.printStackTrace();
13 } finally {
14 this.session.close();
15 }
16 }
18 public DataObject doAnotherFind(String a, String b) {
19 try {
20 this.session = MyHibernateUtil.getSessionFactory().getCurrentSession();
21 Transaction tx = this.session.beginTransaction();
22 //do stuff with session and return a DataObject
23 } catch (Exception ex) {
24 ex.printStackTrace();
25 } finally {
26 this.session.close();
27 }
28 }
30 }
Note how on line 2 of GetAllResponses the MyHome class is instantiated as a static field, and that on lines 8 and 20 of MyHome the field session is assigned.
From my understanding of things, the SSB MyHome is not being managed by the J2EE server (JBoss 4.2.2 GA) as it has been instantiated as a static field of the GetAllResponses class rather than being looked up on JNDI. Therefore, two threads could access the same instance of MyHome at the same time, and because the session is stored in the field session, the first call could very easily have its Session replaced with another Session, causing all sorts of problems including org.hibernate.SessionException: Session is closed! on lines 9 and 21 of MyHome (as an example, two threads call findDataObject, the first thread runs line 14 just after the second thread has run line 8 and before it has run line 9).
Am I correct?
Yes, you're correct.
Also, be aware that even if myHome was not static, all the objects returned by the MyHome would be disconnected from the session. You won't be able to initialized lazy properties from outside of MyHome.
The session variable should also be a local variable in MyHome methods, rather than an instance variable.
But the main problem is that stateless session beans are supposed to be used to demarcate transactions declaratively. In an EJB environment, you shouldn't have to open, commit and rollback transactions, and to close sessions. Everything should be done by the JTA transaction synchronization implemented through sessionFactory.getCurrentSession().