borinquenkid commented on code in PR #16763:
URL: https://github.com/apache/grails-core/pull/16763#discussion_r4418583534
##########
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:
Addressed in commit 1b4eaafcd4:
1. Encapsulated outer session lifecycle management into explicit
OuterSessionLifecycleStrategy implementations
(ActiveTransactionOuterSessionStrategy,
NonTransactionalSynchronizationOuterSessionStrategy,
DefaultOuterSessionStrategy, and NoOpOuterSessionStrategy).
2. Implemented suspend() in
NonTransactionalSynchronizationOuterSessionStrategy to call
logicalConnection.manualDisconnect(). When an inner REQUIRED transaction begins
and Spring suspends outer synchronizations, the primed session drops its
connection immediately, preventing the inner transaction from reusing or
closing it.
3. Implemented beforeCompletion() to disconnect before Spring's
ConnectionSynchronization closes the physical connection when no inner
transaction suspends the scope.
4. Added dedicated unit tests in OuterSessionLifecycleStrategySpec covering
each strategy and interaction invariants in isolation, and added integration
coverage in WithNewSessionLazyLoadSpec for a SUPPORTS scope executing a nested
withNewSession followed by a REQUIRED transaction.
--
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]
