jamesfredley commented on code in PR #16763:
URL: https://github.com/apache/grails-core/pull/16763#discussion_r4396226976
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/GrailsHibernateTemplate.java:
##########
@@ -220,6 +221,55 @@ public <T> T execute(Closure<T> callable) {
public <T> T executeWithNewSession(final Closure<T> callable) {
SessionHolder sessionHolder = (SessionHolder)
txResources.getResource(sessionFactory);
SessionHolder previousHolder = sessionHolder;
+
+ // If the outer session is open but physically disconnected, acquire
its physical connection
+ // before saving previousConnectionHolder and synchronizations, and
before unbinding and entering
+ // the new session scope. Otherwise, lazy loading on outer session
entities inside
+ // withNewSession { withTransaction { ... } } would borrow the inner
transaction's
+ // thread-bound connection and be left with a closed connection when
the inner transaction commits.
+ // con.getAutoCommit() is used because TransactionAwareDataSourceProxy
only obtains a target
+ // connection on its first real method call, binding the proxy to a
target connection.
+ // Unlike getMetaData(), getAutoCommit() allows
LazyConnectionDataSourceProxy to answer
+ // without eagerly checking out a connection from the pool when
nothing is lazy-loaded.
+ if (previousHolder != null && previousHolder.getSession() != null &&
previousHolder.getSession().isOpen()) {
+ Session previousSession = previousHolder.getSession();
+ if (previousSession instanceof SessionImplementor sessionImpl) {
+ JdbcCoordinator jdbcCoordinator =
sessionImpl.getJdbcCoordinator();
+ if (jdbcCoordinator != null) {
+ LogicalConnectionImplementor logicalConnection =
jdbcCoordinator.getLogicalConnection();
+ if (logicalConnection != null &&
!logicalConnection.isPhysicallyConnected()) {
+ Connection con =
logicalConnection.getPhysicalConnection();
+ if (con != null) {
+ try {
+ con.getAutoCommit();
+ } catch (SQLException e) {
+ LOG.warn("Could not initialize physical
connection: {}", e.getMessage(), e);
+ }
+ if (txResources.isSynchronizationActive()) {
+ // When nothing in the scope uses the outer
session, no session synchronization
+ // is registered, and the JDBC
ConnectionSynchronization would close the connection
+ // the outer session still holds. Registering
at SESSION_SYNCHRONIZATION_ORDER ensures
+ // the session is disconnected before Spring's
ConnectionSynchronization runs.
+ txResources.registerSynchronization(new
TransactionSynchronization() {
+ @Override
+ public int getOrder() {
+ return
SessionFactoryUtils.SESSION_SYNCHRONIZATION_ORDER;
+ }
+
+ @Override
+ public void beforeCompletion() {
+ if (logicalConnection.isOpen() &&
logicalConnection.isPhysicallyConnected()) {
+
logicalConnection.manualDisconnect();
Review Comment:
With hibernate.connection.provider_disables_autocommit=true, a
non-transaction-aware data source, and pool autocommit off, an insert after
executeWithNewSession {} commits and leaves zero rows. The base keeps the row.
beforeCompletion calls manualDisconnect before
HibernateTransactionManager.doCommit. Restrict this synchronization to scopes
that are not an actual transaction, and keep the SUPPORTS cleanup.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/GrailsHibernateTemplate.java:
##########
@@ -220,6 +221,55 @@ public <T> T execute(Closure<T> callable) {
public <T> T executeWithNewSession(final Closure<T> callable) {
SessionHolder sessionHolder = (SessionHolder)
txResources.getResource(sessionFactory);
SessionHolder previousHolder = sessionHolder;
+
+ // If the outer session is open but physically disconnected, acquire
its physical connection
+ // before saving previousConnectionHolder and synchronizations, and
before unbinding and entering
+ // the new session scope. Otherwise, lazy loading on outer session
entities inside
+ // withNewSession { withTransaction { ... } } would borrow the inner
transaction's
+ // thread-bound connection and be left with a closed connection when
the inner transaction commits.
+ // con.getAutoCommit() is used because TransactionAwareDataSourceProxy
only obtains a target
+ // connection on its first real method call, binding the proxy to a
target connection.
+ // Unlike getMetaData(), getAutoCommit() allows
LazyConnectionDataSourceProxy to answer
+ // without eagerly checking out a connection from the pool when
nothing is lazy-loaded.
+ if (previousHolder != null && previousHolder.getSession() != null &&
previousHolder.getSession().isOpen()) {
+ Session previousSession = previousHolder.getSession();
+ if (previousSession instanceof SessionImplementor sessionImpl) {
+ JdbcCoordinator jdbcCoordinator =
sessionImpl.getJdbcCoordinator();
+ if (jdbcCoordinator != null) {
+ LogicalConnectionImplementor logicalConnection =
jdbcCoordinator.getLogicalConnection();
+ if (logicalConnection != null &&
!logicalConnection.isPhysicallyConnected()) {
+ Connection con =
logicalConnection.getPhysicalConnection();
Review Comment:
When lazy connection wrapping is off, getPhysicalConnection checks out a
pooled connection even if the outer session is never used. A one-connection
pool then times out on withNewSession. The base query succeeds. Only take this
connection when the transaction-aware proxy workaround needs it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
