From f24b91da2a789b0cb5a3b292b16b2621ee40916a Mon Sep 17 00:00:00 2001 From: boris-unckel Date: Thu, 29 Jul 2021 15:19:17 +0200 Subject: [PATCH] HHH-14763 Avoid suppress exceptions in try/finally --- .../org/hibernate/internal/SessionImpl.java | 26 ++++++++++++++-- .../internal/util/ExceptionHelper.java | 28 +++++++++++++++++ .../DdlTransactionIsolatorNonJtaImpl.java | 30 +++++++++++++++---- .../jta/internal/JtaIsolationDelegate.java | 23 ++++++++------ 4 files changed, 89 insertions(+), 18 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/internal/util/ExceptionHelper.java diff --git a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java index 33e978006b..93f28e3e98 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java @@ -111,6 +111,7 @@ import org.hibernate.graph.GraphSemantic; import org.hibernate.graph.internal.RootGraphImpl; import org.hibernate.graph.spi.RootGraphImplementor; +import org.hibernate.internal.util.ExceptionHelper; import org.hibernate.jpa.AvailableSettings; import org.hibernate.jpa.QueryHints; import org.hibernate.jpa.internal.util.CacheModeHelper; @@ -712,6 +713,7 @@ public void persist(String entityName, Object object, Map copiedAlready) throws } private void firePersist(final PersistEvent event) { + Throwable originalException = null; try { checkTransactionSynchStatus(); checkNoUnresolvedActionsBeforeOperation(); @@ -719,18 +721,36 @@ private void firePersist(final PersistEvent event) { fastSessionServices.eventListenerGroup_PERSIST.fireEventOnEachListener( event, PersistEventListener::onPersist ); } catch (MappingException e) { - throw getExceptionConverter().convert( new IllegalArgumentException( e.getMessage() ) ); + originalException = getExceptionConverter().convert( new IllegalArgumentException( e.getMessage() ) ); } catch (RuntimeException e) { - throw getExceptionConverter().convert( e ); + originalException = getExceptionConverter().convert( e ); + } + catch (Throwable t1) { + originalException = t1; } finally { + Throwable suppressed = null; try { checkNoUnresolvedActionsAfterOperation(); } catch (RuntimeException e) { - throw getExceptionConverter().convert( e ); + suppressed = getExceptionConverter().convert( e ); } + catch (Throwable t2) { + suppressed = t2; + } + if ( suppressed != null ) { + if ( originalException == null ) { + originalException = suppressed; + } + else { + originalException.addSuppressed( suppressed ); + } + } + } + if ( originalException != null ) { + ExceptionHelper.doThrow( originalException ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/ExceptionHelper.java b/hibernate-core/src/main/java/org/hibernate/internal/util/ExceptionHelper.java new file mode 100644 index 0000000000..539ec90e8f --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/internal/util/ExceptionHelper.java @@ -0,0 +1,28 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.internal.util; + + +public final class ExceptionHelper { + + private ExceptionHelper() { + } + + /** + * Throws the given throwable even if it is a checked exception. + * + * @param e The throwable to throw. + */ + public static void doThrow(Throwable e) { + ExceptionHelper.doThrow0(e); + } + + @SuppressWarnings("unchecked") + private static void doThrow0(Throwable e) throws T { + throw (T) e; + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jdbc/internal/DdlTransactionIsolatorNonJtaImpl.java b/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jdbc/internal/DdlTransactionIsolatorNonJtaImpl.java index 3cee311244..ec14cc9795 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jdbc/internal/DdlTransactionIsolatorNonJtaImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jdbc/internal/DdlTransactionIsolatorNonJtaImpl.java @@ -10,6 +10,7 @@ import java.sql.SQLException; import org.hibernate.internal.log.ConnectionAccessLogger; +import org.hibernate.internal.util.ExceptionHelper; import org.hibernate.resource.transaction.spi.DdlTransactionIsolator; import org.hibernate.tool.schema.internal.exec.JdbcContext; @@ -81,29 +82,46 @@ public Connection getIsolatedConnection() { @Override public void release() { if ( jdbcConnection != null ) { + Throwable originalException = null; try { if ( unsetAutoCommit ) { try { jdbcConnection.setAutoCommit( false ); } catch (SQLException e) { - throw jdbcContext.getSqlExceptionHelper().convert( + originalException = jdbcContext.getSqlExceptionHelper().convert( e, - "Unable to set auto commit to false for JDBC Connection used for DDL execution" - ); + "Unable to set auto commit to false for JDBC Connection used for DDL execution" ); + } + catch (Throwable t1) { + originalException = t1; } } } finally { + Throwable suppressed = null; try { jdbcContext.getJdbcConnectionAccess().releaseConnection( jdbcConnection ); } catch (SQLException e) { - throw jdbcContext.getSqlExceptionHelper().convert( + suppressed = jdbcContext.getSqlExceptionHelper().convert( e, - "Unable to release JDBC Connection used for DDL execution" - ); + "Unable to release JDBC Connection used for DDL execution" ); } + catch (Throwable t2) { + suppressed = t2; + } + if ( suppressed != null ) { + if ( originalException == null ) { + originalException = suppressed; + } + else { + originalException.addSuppressed( suppressed ); + } + } + } + if ( originalException != null ) { + ExceptionHelper.doThrow( originalException ); } } } diff --git a/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jta/internal/JtaIsolationDelegate.java b/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jta/internal/JtaIsolationDelegate.java index bab5eac250..2fba3356f7 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jta/internal/JtaIsolationDelegate.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jta/internal/JtaIsolationDelegate.java @@ -20,6 +20,7 @@ import org.hibernate.engine.transaction.spi.IsolationDelegate; import org.hibernate.internal.CoreLogging; import org.hibernate.internal.CoreMessageLogger; +import org.hibernate.internal.util.ExceptionHelper; import org.hibernate.jdbc.WorkExecutor; import org.hibernate.jdbc.WorkExecutorVisitable; @@ -103,36 +104,40 @@ public T call() throws HibernateException { } private T doInSuspendedTransaction(HibernateCallable callable) { + Throwable originalException = null; try { // First we suspend any current JTA transaction Transaction surroundingTransaction = transactionManager.suspend(); LOG.debugf( "Surrounding JTA transaction suspended [%s]", surroundingTransaction ); - boolean hadProblems = false; try { return callable.call(); } - catch (HibernateException e) { - hadProblems = true; - throw e; + catch (Throwable t1) { + originalException = t1; } finally { try { transactionManager.resume( surroundingTransaction ); LOG.debugf( "Surrounding JTA transaction resumed [%s]", surroundingTransaction ); } - catch (Throwable t) { + catch (Throwable t2) { // if the actually work had an error use that, otherwise error based on t - if ( !hadProblems ) { - //noinspection ThrowFromFinallyBlock - throw new HibernateException( "Unable to resume previously suspended transaction", t ); + if ( originalException == null ) { + originalException = new HibernateException( "Unable to resume previously suspended transaction", t2 ); + } + else { + originalException.addSuppressed( t2 ); // No extra nesting, directly t2 } } } } catch (SystemException e) { - throw new HibernateException( "Unable to suspend current JTA transaction", e ); + originalException = new HibernateException( "Unable to suspend current JTA transaction", e ); } + + ExceptionHelper.doThrow( originalException ); + return null; } private T doInNewTransaction(HibernateCallable callable, TransactionManager transactionManager) {