Skip to content

Commit

Permalink
Explicitly release rolled-back database savepoints during (long-runni…
Browse files Browse the repository at this point in the history
…ng) transaction

Issue: SPR-12228
  • Loading branch information
jhoeller committed Sep 22, 2014
1 parent bf93f0c commit 9c8f7d9
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
Expand Up @@ -120,12 +120,19 @@ public Object createSavepoint() throws TransactionException {
*/
@Override
public void rollbackToSavepoint(Object savepoint) throws TransactionException {
ConnectionHolder conHolder = getConnectionHolderForSavepoint();
try {
getConnectionHolderForSavepoint().getConnection().rollback((Savepoint) savepoint);
conHolder.getConnection().rollback((Savepoint) savepoint);

This comment has been minimized.

Copy link
@romank0

romank0 Sep 22, 2014

I think this fix is incorrect as it will introduces discrepancy between database notion of rollback to savepoint and JdbcTransactionObjectSupport (and its clients) view of rollback to savepoint. That is this fix does not allow replicate scenario absolutely valid in database:

BEGIN;
SAVEPOINT A;
ROLLBACK TO SAVEPOINT A;
ROLLBACK TO SAVEPOINT A;
COMMIT;

I believe the correct fix is like this romank0@e2b7b2a
The reason I didn't create PR for this is the bug in openjpa rollbackToSavepoint which is used for integration tests. The bug prevents this fix from working.

}
catch (Throwable ex) {
throw new TransactionSystemException("Could not roll back to JDBC savepoint", ex);
}
try {
conHolder.getConnection().releaseSavepoint((Savepoint) savepoint);
}
catch (Throwable ex) {
logger.debug("Could not explicitly release JDBC savepoint after rollback", ex);
}
}

/**
Expand All @@ -134,8 +141,9 @@ public void rollbackToSavepoint(Object savepoint) throws TransactionException {
*/
@Override
public void releaseSavepoint(Object savepoint) throws TransactionException {
ConnectionHolder conHolder = getConnectionHolderForSavepoint();
try {
getConnectionHolderForSavepoint().getConnection().releaseSavepoint((Savepoint) savepoint);
conHolder.getConnection().releaseSavepoint((Savepoint) savepoint);
}
catch (Throwable ex) {
logger.debug("Could not explicitly release JDBC savepoint", ex);
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2014 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -21,13 +21,13 @@
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.sql.Savepoint;

import javax.sql.DataSource;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.InOrder;

import org.springframework.dao.DataAccessResourceFailureException;
import org.springframework.jdbc.UncategorizedSQLException;
import org.springframework.jdbc.support.nativejdbc.SimpleNativeJdbcExtractor;
Expand Down Expand Up @@ -57,9 +57,12 @@
public class DataSourceTransactionManagerTests {

private Connection con;

private DataSource ds;

private DataSourceTransactionManager tm;


@Before
public void setUp() throws Exception {
con = mock(Connection.class);
Expand All @@ -76,6 +79,7 @@ public void verifyTransactionSynchronizationManagerState() {
assertFalse(TransactionSynchronizationManager.isActualTransactionActive());
}


@Test
public void testTransactionCommitWithAutoCommitTrue() throws Exception {
doTestTransactionCommitRestoringAutoCommit(true, false, false);
Expand Down Expand Up @@ -1290,6 +1294,7 @@ protected void doInTransactionWithoutResult(TransactionStatus status) throws Run

assertTrue("Hasn't thread connection", !TransactionSynchronizationManager.hasResource(ds));
verify(con).rollback(sp);
verify(con).releaseSavepoint(sp);
verify(con).commit();
verify(con).isReadOnly();
verify(con).close();
Expand Down Expand Up @@ -1395,14 +1400,19 @@ protected void doInTransactionWithoutResult(TransactionStatus status) throws Run
verify(con).close();
}


private static class TestTransactionSynchronization implements TransactionSynchronization {

private DataSource dataSource;

private int status;

public boolean beforeCommitCalled;

public boolean beforeCompletionCalled;

public boolean afterCommitCalled;

public boolean afterCompletionCalled;

public TestTransactionSynchronization(DataSource dataSource, int status) {
Expand Down

0 comments on commit 9c8f7d9

Please sign in to comment.