Skip to content

Commit f24b91d

Browse files
boris-unckelbeikov
authored andcommitted
HHH-14763 Avoid suppress exceptions in try/finally
1 parent 6314395 commit f24b91d

File tree

4 files changed

+89
-18
lines changed

4 files changed

+89
-18
lines changed

hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@
111111
import org.hibernate.graph.GraphSemantic;
112112
import org.hibernate.graph.internal.RootGraphImpl;
113113
import org.hibernate.graph.spi.RootGraphImplementor;
114+
import org.hibernate.internal.util.ExceptionHelper;
114115
import org.hibernate.jpa.AvailableSettings;
115116
import org.hibernate.jpa.QueryHints;
116117
import org.hibernate.jpa.internal.util.CacheModeHelper;
@@ -712,26 +713,45 @@ public void persist(String entityName, Object object, Map copiedAlready) throws
712713
}
713714

714715
private void firePersist(final PersistEvent event) {
716+
Throwable originalException = null;
715717
try {
716718
checkTransactionSynchStatus();
717719
checkNoUnresolvedActionsBeforeOperation();
718720

719721
fastSessionServices.eventListenerGroup_PERSIST.fireEventOnEachListener( event, PersistEventListener::onPersist );
720722
}
721723
catch (MappingException e) {
722-
throw getExceptionConverter().convert( new IllegalArgumentException( e.getMessage() ) );
724+
originalException = getExceptionConverter().convert( new IllegalArgumentException( e.getMessage() ) );
723725
}
724726
catch (RuntimeException e) {
725-
throw getExceptionConverter().convert( e );
727+
originalException = getExceptionConverter().convert( e );
728+
}
729+
catch (Throwable t1) {
730+
originalException = t1;
726731
}
727732
finally {
733+
Throwable suppressed = null;
728734
try {
729735
checkNoUnresolvedActionsAfterOperation();
730736
}
731737
catch (RuntimeException e) {
732-
throw getExceptionConverter().convert( e );
738+
suppressed = getExceptionConverter().convert( e );
739+
}
740+
catch (Throwable t2) {
741+
suppressed = t2;
742+
}
743+
if ( suppressed != null ) {
744+
if ( originalException == null ) {
745+
originalException = suppressed;
746+
}
747+
else {
748+
originalException.addSuppressed( suppressed );
749+
}
733750
}
734751
}
752+
if ( originalException != null ) {
753+
ExceptionHelper.doThrow( originalException );
754+
}
735755
}
736756

737757
private void firePersist(final Map copiedAlready, final PersistEvent event) {
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Hibernate, Relational Persistence for Idiomatic Java
3+
*
4+
* License: GNU Lesser General Public License (LGPL), version 2.1 or later.
5+
* See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
6+
*/
7+
package org.hibernate.internal.util;
8+
9+
10+
public final class ExceptionHelper {
11+
12+
private ExceptionHelper() {
13+
}
14+
15+
/**
16+
* Throws the given throwable even if it is a checked exception.
17+
*
18+
* @param e The throwable to throw.
19+
*/
20+
public static void doThrow(Throwable e) {
21+
ExceptionHelper.<RuntimeException>doThrow0(e);
22+
}
23+
24+
@SuppressWarnings("unchecked")
25+
private static <T extends Throwable> void doThrow0(Throwable e) throws T {
26+
throw (T) e;
27+
}
28+
}

hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jdbc/internal/DdlTransactionIsolatorNonJtaImpl.java

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import java.sql.SQLException;
1111

1212
import org.hibernate.internal.log.ConnectionAccessLogger;
13+
import org.hibernate.internal.util.ExceptionHelper;
1314
import org.hibernate.resource.transaction.spi.DdlTransactionIsolator;
1415
import org.hibernate.tool.schema.internal.exec.JdbcContext;
1516

@@ -81,29 +82,46 @@ public Connection getIsolatedConnection() {
8182
@Override
8283
public void release() {
8384
if ( jdbcConnection != null ) {
85+
Throwable originalException = null;
8486
try {
8587
if ( unsetAutoCommit ) {
8688
try {
8789
jdbcConnection.setAutoCommit( false );
8890
}
8991
catch (SQLException e) {
90-
throw jdbcContext.getSqlExceptionHelper().convert(
92+
originalException = jdbcContext.getSqlExceptionHelper().convert(
9193
e,
92-
"Unable to set auto commit to false for JDBC Connection used for DDL execution"
93-
);
94+
"Unable to set auto commit to false for JDBC Connection used for DDL execution" );
95+
}
96+
catch (Throwable t1) {
97+
originalException = t1;
9498
}
9599
}
96100
}
97101
finally {
102+
Throwable suppressed = null;
98103
try {
99104
jdbcContext.getJdbcConnectionAccess().releaseConnection( jdbcConnection );
100105
}
101106
catch (SQLException e) {
102-
throw jdbcContext.getSqlExceptionHelper().convert(
107+
suppressed = jdbcContext.getSqlExceptionHelper().convert(
103108
e,
104-
"Unable to release JDBC Connection used for DDL execution"
105-
);
109+
"Unable to release JDBC Connection used for DDL execution" );
110+
}
111+
catch (Throwable t2) {
112+
suppressed = t2;
106113
}
114+
if ( suppressed != null ) {
115+
if ( originalException == null ) {
116+
originalException = suppressed;
117+
}
118+
else {
119+
originalException.addSuppressed( suppressed );
120+
}
121+
}
122+
}
123+
if ( originalException != null ) {
124+
ExceptionHelper.doThrow( originalException );
107125
}
108126
}
109127
}

hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jta/internal/JtaIsolationDelegate.java

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.hibernate.engine.transaction.spi.IsolationDelegate;
2121
import org.hibernate.internal.CoreLogging;
2222
import org.hibernate.internal.CoreMessageLogger;
23+
import org.hibernate.internal.util.ExceptionHelper;
2324
import org.hibernate.jdbc.WorkExecutor;
2425
import org.hibernate.jdbc.WorkExecutorVisitable;
2526

@@ -103,36 +104,40 @@ public T call() throws HibernateException {
103104
}
104105

105106
private <T> T doInSuspendedTransaction(HibernateCallable<T> callable) {
107+
Throwable originalException = null;
106108
try {
107109
// First we suspend any current JTA transaction
108110
Transaction surroundingTransaction = transactionManager.suspend();
109111
LOG.debugf( "Surrounding JTA transaction suspended [%s]", surroundingTransaction );
110112

111-
boolean hadProblems = false;
112113
try {
113114
return callable.call();
114115
}
115-
catch (HibernateException e) {
116-
hadProblems = true;
117-
throw e;
116+
catch (Throwable t1) {
117+
originalException = t1;
118118
}
119119
finally {
120120
try {
121121
transactionManager.resume( surroundingTransaction );
122122
LOG.debugf( "Surrounding JTA transaction resumed [%s]", surroundingTransaction );
123123
}
124-
catch (Throwable t) {
124+
catch (Throwable t2) {
125125
// if the actually work had an error use that, otherwise error based on t
126-
if ( !hadProblems ) {
127-
//noinspection ThrowFromFinallyBlock
128-
throw new HibernateException( "Unable to resume previously suspended transaction", t );
126+
if ( originalException == null ) {
127+
originalException = new HibernateException( "Unable to resume previously suspended transaction", t2 );
128+
}
129+
else {
130+
originalException.addSuppressed( t2 ); // No extra nesting, directly t2
129131
}
130132
}
131133
}
132134
}
133135
catch (SystemException e) {
134-
throw new HibernateException( "Unable to suspend current JTA transaction", e );
136+
originalException = new HibernateException( "Unable to suspend current JTA transaction", e );
135137
}
138+
139+
ExceptionHelper.doThrow( originalException );
140+
return null;
136141
}
137142

138143
private <T> T doInNewTransaction(HibernateCallable<T> callable, TransactionManager transactionManager) {

0 commit comments

Comments
 (0)