Bill Treuman and Igor Dayen have responded to reader feedback generated by their article on using Hibernate 3.1 with Tomcat 5.5, posted last month, with a newly revised and updated version. This new article explains the authors' chosen programming style.

Our objective is to teach and illustrate how Hibernate 3.1 and Tomcat 5.5 function together (which is after all the title of this series). Code which shows its innards because it is meant to be easy to understand, like this code, is not the same as commercial quality code you intend to deploy - which would include complete exception handling and other practices such as use of the MVC pattern which are irrelevant to our purposes. (You would certainly use a JSP, not a servlet, as your View component. But introducing even part of an MVC pattern would blur the focus from where we wish it to stay - the intersection of Hibernate and Tomcat.)

In addition, the article now describes options for saving information in a shopping cart scenario comprised of multiple Http requests. Also, an instance variable has now correctly been reposted as a local variable declared within a method, so the program can work under multi-user testing. What are your thoughts on this revised article, and on using Hibernate and Tomcat together?

Session handling (the code leaks) and the conceptual advice about how the Hibernate Session should be used are still completely wrong. The authors still don't understand how a thread-local works in the context of Hibernate Session management. This article is as bad as it was before. It's OK if you take the Hibernate reference documentation tutorial, rewrite it with a few different examples, but keep the major points intact. However, adding various quotes (verbatim) from Hibernate in Action, out of context, is not OK if the end result is misleading and simply not correct. Really, why couldn't the authors just copy everything from the reference tutorial and only replace only the Tomcat configuration part?

So, could you please enlighten us and explain (or point us to articles) on how it is done properly? Thanks a lot. Hibernate rocks!

http://hibernate.org/hib_docs/v3/reference/en/html/tutorial.html http://hibernate.org/hib_docs/v3/reference/en/html/transactions.html http://www.manning.com/bauer http://www.manning.com/bauer2

So, could you please enlighten us and explain (or point us to articles) on how it is done properly?

Thanks a lot.
Hibernate rocks!

Per his post. http://hibernate.org/hib_docs/v3/reference/en/html/transactions.html look at the section 11.1.1 - Unit Of Work In most cases, you have to uses sessionFactory.getCurrentSession() which sticks it in ThreadLocal if you aren't using JTA. Christian Bauer stated this - [blockquote] Another thought: Could it be that you don't know that the scope of a value in a thread-local variable is a single request? I'm guessing here, but from what you wrote it sounds like you have tried to store something in a thread-local and expected it to be there for several requests. And then came the thread pool etc...[/blockquote] sessionFactory.getCurrentSession() sticks information in the thread local if it's not using JTA. Even though tomcat has worker threads, when it's returned to the pool, ThreadLocal is effectively reset before it handles another request.

Even though tomcat has worker threads, when it's returned to the pool, ThreadLocal is effectively reset before it handles another request.

In regards to the session-per-request pattern. I'm not sure if sessionFactory.getCurrentSession() uses a ThreadLocal (I believe most JTA implementations uses ThreadLocals). I guess this is why I use spring which automatically cleans things up.

Even though tomcat has worker threads, when it's returned to the pool, ThreadLocal is effectively reset before it handles another request.

Not in my experience. In the early stages of development we were using a servlet filter + ThreadLocal to manage the hibernate session. The filter mapping was accidentally changed and didn't execute. Took a while to spot because good old Tomcat re-used the thread 3 or 4 times across requests and the session still existed. A few requests later we'd get a different thread and with an old session and lose all our updates. Can't remember which version of Tomcat (about 2 yrs ago / sure it was 5.something).

Even though tomcat has worker threads, when it's returned to the pool, ThreadLocal is effectively reset before it handles another request.

Not in my experience. In the early stages of development we were using a servlet filter + ThreadLocal to manage the hibernate session. The filter mapping was accidentally changed and didn't execute. Took a while to spot because good old Tomcat re-used the thread 3 or 4 times across requests and the session still existed. A few requests later we'd get a different thread and with an old session and lose all our updates.

Since the Java class library doesn't provide a "Thread.removeAllThreadLocalVars()" method or anything close to that, there's no way to reset all the ThreadLocals when a thread is placed back into a pool to be reused later. There's also no way to query the Thread for all its ThreadLocals. The code that's using the ThreadLocal is responsible for removing it from the thread when it's done with it. There's no way for a central service (e.g. an app server or its ThreadPool) to magically do this.

Since the Java class library doesn't provide a "Thread.removeAllThreadLocalVars()" method or anything close to that, there's no way to reset all the ThreadLocals when a thread is placed back into a pool to be reused later. There's also no way to query the Thread for all its ThreadLocals. The code that's using the ThreadLocal is responsible for removing it from the thread when it's done with it. There's no way for a central service (e.g. an app server or its ThreadPool) to magically do this.

But in terms of hibernate, the docs state that if you are using sessionFactory.getCurrentSession()(and it's configured to use ThreadLocalSessionContext which probably makes sense if one is just using Tomcat 5.5 and not a full server with JTA), then a call to sessionFactory.getCurrentSession().close() in a servlet filter should clean things up properly in a session-per-request strategy (user just has to call it, Hibernate knows what ThreadLocal vars it created and will clean it properly). Per the Hibernate API docs on the ThreadLocalSessionContext

A CurrentSessionContext impl which scopes the notion of current session by the current thread of execution. Unlike the JTA counterpart, threads do not give us a nice hook to perform any type of cleanup making it questionable for this impl to actually generate Session instances. In the interest of usability, it was decided to have this default impl actually generate a session upon first request and then clean it up after the Transaction associated with that session is committed/rolled-back. In order for ensuring that happens, the sessions generated here are unusable until after Session.beginTransaction() has been called. If close() is called on a session managed by this class, it will be automatically unbound.

A CurrentSessionContext impl which scopes the notion of current session by the current thread of execution. Unlike the JTA counterpart, threads do not give us a nice hook to perform any type of cleanup making it questionable for this impl to actually generate Session instances. In the interest of usability, it was decided to have this default impl actually generate a session upon first request and then clean it up after the Transaction associated with that session is committed/rolled-back. In order for ensuring that happens, the sessions generated here are unusable until after Session.beginTransaction() has been called. If close() is called on a session managed by this class, it will be automatically unbound.

The close() is not necessary: "and then clean it up after the Transaction associated with that Session is committed/rolled back". The Javadoc simply says what happens if you call close() (for whatever reason). The idiom for session-per-request with TLSContext is: sf.getCurrentSession().beginTransaction(); sf.getCurrentSession().load(); sf.getCurrentSession().save(); sf.getCurrentSession().getTransaction().commit(); What the authors didn't understand is that this has nothing to do with the Session you obtain from sf.openSession(). You have to close() it. See http://hibernate.org/42.html And what you are proposing is implemented in the Hibernate documentation reference tutorial and on http://hibernate.org/43.html

Thanks for pointing it out. It was news to me that session can be automatically closed when calling "openSession". So the author probably needs to modify his article to use "getCurrentSession"(with the right configuration).

It was news to me that session can be automatically closed when calling "openSession". *sigh* It isn't. If you open a Session, you close it. Not Hibernate. (Yes, there are some obscure configuration switches for JTA environments where Hibernate registers a Synchronization with the TransactionManger for automatic closing. With the introduction of getCurrentSession() these are obsolete.)

I've again updated http://hibernate.org/42.html and now copied more example code from the Hibernate reference documentation (nobody seems to read that anymore) to the Wiki. Hopefully that explains the three confusing methods openSession(), getCurrentSession(), and close() in enough detail.

"If you manage the Session yourself, code is more difficult to layer. You can't easily move data access operations into a different layer than transaction and Session demarcation." I can't understand it. Are there any examples in the Hibernate reference documentation?? If I use getCurrentSession(), it's easier to move??

A CurrentSessionContext impl which scopes the notion of current session by the current thread of execution. Unlike the JTA counterpart, threads do not give us a nice hook to perform any type of cleanup making it questionable for this impl to actually generate Session instances. In the interest of usability, it was decided to have this default impl actually generate a session upon first request and then clean it up after the Transaction associated with that session is committed/rolled-back. In order for ensuring that happens, the sessions generated here are unusable until after Session.beginTransaction() has been called. If close() is called on a session managed by this class, it will be automatically unbound.

The close() is not necessary: "and then clean it up after the Transaction associated with that Session is committed/rolled back". The Javadoc simply says what happens if you call close() (for whatever reason). The idiom for session-per-request with TLSContext is:

sf.getCurrentSession().beginTransaction();

sf.getCurrentSession().load();
sf.getCurrentSession().save();

sf.getCurrentSession().getTransaction().commit();

What the authors didn't understand is that this has nothing to do with the Session you obtain from sf.openSession(). You have to close() it. See http://hibernate.org/42.html

And what you are proposing is implemented in the Hibernate documentation reference tutorial and on http://hibernate.org/43.html

So does this eventually mean.... a. You don't need to write your own ThreadLocal implementation to manage the sessions, since hibernate will do it for you if you use getCurrentSession() b. You're still better off using it since the threads created by the tomcat thread pool are not guaranteed to be cleaned up, and hence may end up using one of the old cached session. Albert.

Even though tomcat has worker threads, when it's returned to the pool, ThreadLocal is effectively reset before it handles another request.

Not in my experience. In the early stages of development we were using a servlet filter + ThreadLocal to manage the hibernate session. The filter mapping was accidentally changed and didn't execute. Took a while to spot because good old Tomcat re-used the thread 3 or 4 times across requests and the session still existed. A few requests later we'd get a different thread and with an old session and lose all our updates.

Can't remember which version of Tomcat (about 2 yrs ago / sure it was 5.something).

Considering the authors were going for the really simple approach, why not just do the following void doPost(HttpServletRequest request, HttpServletResponse response){ Session session = sessionFactory.getCurrentSession(); try { ServletContext ctx = getServletConfig().getServletContext(); // do your business logic here, your // service layer, business logic should retrieve // the session via sf.getCurrentSession() if it // requires PROPOGATION_REQUIRED, PROPOGATION_SUPPORTS // or similar semantics // forward to JSP here RequestDispatcher rd = ctx.getRequestDispatcher("/someDisplay.jsp"); rd.forward(request,response); } finally { // This will make sure ThreadLocal vars // created by HB will get cleaned up sessionFactory.getCurrentSession().close(); } really simple, no servlet filters required to make sure the currentSession() is closed..

Ugh. This is completely bogus. http://hibernate.org/42.html http://hibernate.org/43.html And also the reference documentation tutorial...

Session handling (the code leaks) and the conceptual advice about how the Hibernate Session should be used are still completely wrong. The authors still don't understand how a thread-local works in the context of Hibernate Session management. This article is as bad as it was before

When the authors don't understand it how should Joe Programmer get it?

When the authors don't understand it how should Joe Programmer get it?

Exactly.

Have to agree (It's still bad), and not just because of how you use hibernate, e.g.

rs.close(); stmt.close(); conn.close(); initCtx.close();

Lines 1 and 2 are unnecessary (unless there is significant delay beween closing the rs / stmt and the connection). Line 3 should be in a finally block otherwise you risk leaking connections. Line 4 should be executed in a separate finally block as soon as you've obtained the JNDI resource. Currently if line 3 throws an exception line 4 will never execute. My first hibernate related critism is that opening a new hibernate session every request is often a poor approach for updates. It prevents transparent use of hibernate's version checking and you will require an otherwise unecessary db call. I generally prefer the long transaction alternative you refer to, but accept your points regarding scalability (for large object graphs) and potential issues with clustering (sticky sessions are usually a good enough solution). Secondly I think your problems with Tomcat's re-use of threads may be because you are not closing the hibernate session properly. I've not used ThreadLocalSessionContext and the javadoc isn't sufficiently explicit re "clean up" after commit / rollback, however if the session had been closed it doesn't make sense that hiberate would return the same instance in future and allow it to be used. Also your session.getTransaction().commit() line is not in a finally block, so even if ThreadLocalSessionContext does work as you expect, on exception your session won't be closed. Regarding Tomcat's re-use of threads - even if Tomcat didn't re-use threads you shouldn't entertain the idea of writing code that depends on this behavior since it's not standardised by J2EE and you would make your code less portable. I'm also not keen on performing selects in a transaction. I couldn't find the reference in the hibernate docs that says you should do this, but quite believe it's there since hibernate may flush the session before some queries. I prefer to turn this behaviour off, but I don't know whether "flush before select" is the only reason to wrap queries in a transaction. I'm interested in other peoples views on this. The following is a a general point that's not relevant to the goal of your article, but worth mentioning as IMHO your code examples are too "noisy". Instead of directly invoking JNDI I suggest using Spring to obtain the hibernate session. You can still use JNDI from within Spring if desired, but it's probably not necessary. As a side note - with Spring 2 you use can use the scope attribute to associate your hibernate session with the http session, simplifying long transactions. I also like to use AOP to manage my hibernate session and db transactions, thereby removing laborious beginTx, commit and rollback code from my service classes. Finally (and again not relevant to hibernate + tomcat), your Log4J usage demonstrates some extremely bad practice (poor choice of logger names, debug statements not wrapped by isDebugEnabled, debug used for exceptions, use of println). If you're going to complicate your examples with log statements you should at least use them properly. The following shameless plug may help ;) http://www.javaanswers.com/viewtopic.php?t=3

rs.close();
stmt.close();
conn.close();
initCtx.close();

Lines 1 and 2 are unnecessary (unless there is significant delay beween closing the rs / stmt and the connection). Line 3 should be in a finally block otherwise you risk leaking connections. Line 4 should be executed in a separate finally block as soon as you've obtained the JNDI resource.

I totally agree on the fact that each statement should be in an independent try/catch to ensure that each should be executed, but I long ago learnt the habit of writing: try { rs.close() } catch (Exception e) { /* Feh */ } try { ps.close() } catch (Exception e) { /* Feh */ } try { conn.close() } catch (Exception e) { /* Feh */ } Why? Maybe it's no longer the case, but when I started with JDBC it seemed fairly common that pool drivers would leak statements. I.e. unless you explicitly closed your own statements, they'd hang around cached on the connection when you returned the connection to the pool via conn.close(). At any rate, there's nothing inherently wrong with calling close on the statement and result set - you're just doing work that the drvier *should* do when you call connection.close(). -Tim Fennell Stripes: Because web development should just be easier

rs.close();
stmt.close();
conn.close();
initCtx.close();

Lines 1 and 2 are unnecessary (unless there is significant delay beween closing the rs / stmt and the connection). Line 3 should be in a finally block otherwise you risk leaking connections. Line 4 should be executed in a separate finally block as soon as you've obtained the JNDI resource.

I totally agree on the fact that each statement should be in an independent try/catch to ensure that each should be executed, but I long ago learnt the habit of writing:

try { rs.close() } catch (Exception e) { /* Feh */ }
try { ps.close() } catch (Exception e) { /* Feh */ }
try { conn.close() } catch (Exception e) { /* Feh */ }

Why? Maybe it's no longer the case, but when I started with JDBC it seemed fairly common that pool drivers would leak statements. I.e. unless you explicitly closed your own statements, they'd hang around cached on the connection when you returned the connection to the pool via conn.close(). At any rate, there's nothing inherently wrong with calling close on the statement and result set - you're just doing work that the drvier *should* do when you call connection.close().

Even the new JDBC drivers, over long lived connection use, you can end up with too many open pointer errors in different dbs if you do not close your resultsets. So just assuming things are cleaned up by [wrapper] implementations and drivers when you close the connection doesn't guarantee that all the garbage created will be released with JDBC use.

Even the new JDBC drivers, over long lived connection use, you can end up with too many open pointer errors in different dbs if you do not close your resultsets. So just assuming things are cleaned up by [wrapper] implementations and drivers when you close the connection doesn't guarantee that all the garbage created will be released with JDBC use.

From the JDBC 3.0 spec - http://java.sun.com/products/jdbc/download.html#corespec30 "All Statement objects will be closed when the connection that created them is closed." "A ResultSet object is automatically closed when the Statement object that produced it is closed." "Connection pooling is completely transparent to the client: A client obtains a pooled connection and uses it just the same way it obtains and uses a nonpooled connection." That's not to say that the vendors will always implement the JDBC drivers / connection pool wrappers correctly.

try { rs.close() } catch (Exception e) { /* Feh */ } try { ps.close() } catch (Exception e) { /* Feh */ } try { conn.close() } catch (Exception e) { /* Feh */ }

Jakarta commons dbutils makes this a bit nicer.

"All Statement objects will be closed when the connection that created them is closed." "A ResultSet object is automatically closed when the Statement object that produced it is closed." That's not to say that the vendors will always implement the JDBC drivers correctly.

That is really what a TCK is supposed to test (compliance with statements out of the spec), and hence people could claim JDBC version X compliance if they pass the JDBC TCK ...

I totally agree on the fact that each statement should be in an independent try/catch to ensure that each should be executed, but I long ago learnt the habit of writing:

try { rs.close() } catch (Exception e) { /* Feh */ } try { ps.close() } catch (Exception e) { /* Feh */ } try { conn.close() } catch (Exception e) { /* Feh */ }

Wrong! Always release resources in finally. Release more than on resource in nested try/finally blocks. That's the only way it works.

I'm also not keen on performing selects in a transaction. I couldn't find the reference in the hibernate docs that says you should do this, but quite believe it's there since hibernate may flush the session before some queries. I prefer to turn this behaviour off, but I don't know whether "flush before select" is the only reason to wrap queries in a transaction. I'm interested in other peoples views on this.

You would do it for isolation purpose maybe ?
Anyway, as far as I know, even if you don't explicitly start a transaction yourself, the driver (or the database) will do it by itself for every statement you execute.
On the other hand, I often consider the start of a transaction as a snapshot of my database at a given time: all reads I perform in the scope of the transaction will represent the state of my data at the time the transaction was started (although this depends on the isolation level you choose). In any case, this is a way to garantee the consistency of the data you fetch during the lifetime of your transaction.

All -- I enjoyed reading the comments and great discussion and encourage all to keep it up. I also want to encourage article authors to revise articles like this more frequently. I'd much rather have a "live" version (like the books Spring Live, etc.) of an article that's revised with fixes, clarifications, enhancements, etc. When it's warranted, it's worth it, both for the reader's and author's sake. TSS -- when an article gets a lot of response, especially questions or requests for fixes, please contact the author and see if they can revise. Much of a typical Java developer's reference is articles on the net, so keeping them high quality is a challenge that needs to be met. Keep up the good work, editors!

TSS needs some sort of article incubator for these types of articles. It is one thing when a narrow point of an article is debated it is another issue when the entire basis is off kilter.

I'm new to this thread and have read a lot of it with great interest. Is the code available so I can download it and play with it? I'd rather do that then cutting and pasting from the article Thanks Phil Willemann

The code is attached at the bottom of the article.

I am confused the Resource Name parameter. In your Context.xml example, you have ... <Resource name="jdbc/Sampdb" auth=Container... Then when you try to access from the servlet, you use: (DataSource)initCtx.lookup("java:comp/env/jdbc/Sampdb"); My question is, where are these paths coming from? Looking at the ref docs (http://tomcat.apache.org/tomcat-5.0-doc/config/context.html) I see: "You MUST define resource parameters for every resource name that is specified by a element inside a or element in $CATALINA_HOME/conf/server.xml, and/or for every name declared in a or element in the web application deployment descriptor, before that resource can be successfully accessed." Can you please provide the relevant lines in your server.xml file that works with your example? It's true that the example wont run otherwise, correct? Thanks!

Resource name is defined in webapps/BasicWeb/META-INF/Context.xml: <?xml version="1.0" encoding="UTF-8"?>