We're updating the issue view to help you get more done. 

Bug in NonBatchingBatch makes session ununsable after failed transaction

Description

In NonBatchingBatch.addToBatch() are some bugs (missing try...finally) that leaves the class in an undefined condition if the statement there fails. In addition, statements are not closed on error.

I have attached a project that shows the bug and contains a fix.

Here's my complete description (which you will also find in ShowBug.java):

/**

  • Description of the problem:

  • - we have a pre-bound session (as in OpenSessionInViewFilter) - there

  • are multiple transactions - one transaction fails - the transaction

  • manager disconnects the session (this is also true if the transaction

  • succeeds - so it should not be a problem) - according the docs

  • reconnecting should be done by transaction management - all following

  • transactions fail

  • In AbstractBatchImpl: try { doExecuteBatch(); } finally {

  • releaseStatements(); }

  • Statements are released within the same Transaction (ensured via

  • try...finally)

  • In NonBatchingBatch: try { [...] final int rowCount =

  • statement.executeUpdate(); [...] } catch ( SQLException e ) {

  • LOG.debug( "SQLException escaped proxy", e ); throw

  • sqlExceptionHelper().convert( e, "could not execute batch statement",

  • entry.getKey() ); } getStatements().clear();

  • Problems with this implementation: - Statements are not released here

  • - Statements are not closed if the query fails (resource leak!) -

  • statement.clearBatch() is never called (not sure if this matters. at

  • least in AbstractBatchImpl it is called) - Statements are not removed

  • from the list

  • In the next transaction (which succeeds), releaseStatements() is

  • called which fails because the proxy is not valid anymore.

  • Solution:

  • Fix the method NonBatchingBatch.addToBatch() as in attached NonBatchingBatch_fixed.java

  • Details:

  • - move all the close/release stuff outside of the loop

  • - put the loop in a "try" block

  • - in the "finally" block call super.releaseStatements()

  • - in AbstractBatchImpl make releaseStatements() protected instead of private

  • (this should really be done to avoid dupplicate code)

  • */

Environment

None

Status

Assignee

Steve Ebersole

Reporter

Michael Wyraz

Labels

None

Worked in

None

Feedback Requested

None

Feedback Requested By

None

backPortable

None

Community Help Wanted

None

Suitable for new contributors

None

Requires Release Note

Affirmative

Pull Request

None

backportDecision

None

backportReEvaluate

None

Time Tracking

2h

Components

Affects versions

4.1.7

Priority

Major