RE: [Hibernate-devel] not so fast! :)

[Anton v. S. <an...@ap...>]

Hey, these would only be embarrassing if they were caught after v1.0!  To
Hibernate's credit, this latest one at least gave a log warning.  I figured
the external connection scenario wasn't being widely used.  I probably
wouldn't be using it either, except that I'm integrating into an existing
system, and I thought it would be a good idea to share the same connection
pool as the rest of the system.

> I guess this means I have to do another release real soon ... but
> not today.

I agree it's not that urgent, unless you're aware of anyone embedding the
code into heart-lung machines as we speak... :)

> As a high priority, I need to write some
> tests for this functionality.

I was thinking about that: testing the finalize issue would be tricky, since
the symptoms could range from benign to unpredictable.  I think this
particular issue would be handled better by some additional checks (or
assertions).  One would be inside RelationalDatabaseSession.finalize:

	if (conn!=null) {
		log.warn("Unclosed session");
		if (conn.isClosed())
			log.warn("Inconsistent state: unclosed session with closed connection");
		else
			conn.close();
	}

The second warning triggers in my case - that's how I verified what was
going on.

Another place to check for and warn about a non-null connection would be
inside cleanup(), i.e.

	if (conn!=null) log.warn("Unclosed connection");

I believe this would also have triggered in my case, although I didn't try
it.  I guess you could also exercise this kind of thing with some white-box
unit tests.

My first bug, with the parameters swapped around, is easier to unit test
for.  I'll see if I can put something together after I've installed the code
I'm currently working on.  (Unit test after release, XLP - Xtremely Lazy
Programming?)

Anton