jamesfredley commented on code in PR #16763:
URL: https://github.com/apache/grails-core/pull/16763#discussion_r4409717857
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/GrailsHibernateTemplate.java:
##########
@@ -220,6 +232,60 @@ 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.
+ // This workaround is only taken when the connection provider uses
TransactionAwareDataSourceProxy
+ // backed by LazyConnectionDataSourceProxy: con.getAutoCommit() allows
LazyConnectionDataSourceProxy
+ // to bind the proxy without eagerly checking out a connection from
the pool. When lazy wrapping is off
+ // or the data source is non-transaction-aware, acquiring the
connection eagerly would check out a pooled
+ // connection unnecessarily and cause a single-connection pool to time
out.
+ if (this.transactionAware && this.lazyConnection &&
Review Comment:
This priming runs only when lazy wrapping is on. With dataSource.lazy=false
and transactionAware still true, a disconnected outer session is not given its
own connection. A lazy load inside withNewSession { withTransaction { ... } }
then takes the inner transaction's connection, and the outer session keeps that
connection after the inner transaction closes it. The idle outer session must
not check out a pooled connection, and a real use of that session must not
share the inner transaction's connection.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/GrailsHibernateTemplate.java:
##########
@@ -220,6 +232,60 @@ 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.
+ // This workaround is only taken when the connection provider uses
TransactionAwareDataSourceProxy
+ // backed by LazyConnectionDataSourceProxy: con.getAutoCommit() allows
LazyConnectionDataSourceProxy
+ // to bind the proxy without eagerly checking out a connection from
the pool. When lazy wrapping is off
+ // or the data source is non-transaction-aware, acquiring the
connection eagerly would check out a pooled
+ // connection unnecessarily and cause a single-connection pool to time
out.
+ if (this.transactionAware && this.lazyConnection &&
+ 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() &&
!txResources.isActualTransactionActive()) {
+ // When nothing in the scope uses the outer
session in a non-actual-transaction scope
+ // (such as PROPAGATION_SUPPORTS with
synchronization), 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.
+ // We strictly restrict this to scopes that
are NOT an actual transaction to prevent
+ // manualDisconnect() from severing active
write-bearing transactions before doCommit.
+ txResources.registerSynchronization(new
TransactionSynchronization() {
+ @Override
+ public int getOrder() {
+ return
SessionFactoryUtils.SESSION_SYNCHRONIZATION_ORDER;
+ }
+
+ @Override
+ public void beforeCompletion() {
Review Comment:
Inside a SUPPORTS scope, this beforeCompletion does not run when a REQUIRED
transaction suspends that synchronization. The REQUIRED transaction reuses the
connection this scope primed and closes it, so a later query on the outer
session fails with a closed connection. Disconnect the primed session when the
synchronization is suspended.
--
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]
