jamesfredley commented on code in PR #16784:
URL: https://github.com/apache/grails-core/pull/16784#discussion_r4396158964
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/GrailsHibernateTemplate.java:
##########
@@ -153,6 +154,55 @@ public <T> T execute(Closure<T> callable) {
public <T> T executeWithNewSession(final Closure<T> callable) {
SessionHolder sessionHolder = (SessionHolder)
TransactionSynchronizationManager.getResource(sessionFactory);
SessionHolder previousHolder = sessionHolder;
+
+ // If the outer session is disconnected, acquire its physical
connection now before
+ // saving previousConnectionHolder and unbinding to enter the new
session scope.
+ // Otherwise, lazy loading on outer session entities inside
withNewSession { withTransaction { ... } }
+ // would borrow the inner transaction's 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
(TransactionSynchronizationManager.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.
+
TransactionSynchronizationManager.registerSynchronization(new
TransactionSynchronization() {
+ @Override
+ public int getOrder() {
+ return
SessionFactoryUtils.SESSION_SYNCHRONIZATION_ORDER;
+ }
+
Review Comment:
With delayed connection acquisition, an outer transaction can still be
physically disconnected when this scope starts. beforeCompletion then
disconnects that connection before the transaction commits, and a pool that
rolls back a returned connection drops the writes. An insert after
withNewSession committed and left zero rows.
Disconnect only a session that is not in a transaction.
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/GrailsHibernateTemplate.java:
##########
@@ -153,6 +154,55 @@ public <T> T execute(Closure<T> callable) {
public <T> T executeWithNewSession(final Closure<T> callable) {
SessionHolder sessionHolder = (SessionHolder)
TransactionSynchronizationManager.getResource(sessionFactory);
SessionHolder previousHolder = sessionHolder;
+
+ // If the outer session is disconnected, acquire its physical
connection now before
+ // saving previousConnectionHolder and unbinding to enter the new
session scope.
+ // Otherwise, lazy loading on outer session entities inside
withNewSession { withTransaction { ... } }
+ // would borrow the inner transaction's 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();
Review Comment:
dataSource.lazy=false does not install LazyConnectionDataSourceProxy, so
getAutoCommit checks out a real connection for an outer session the closure
never uses. With one connection in the pool, the inner transaction waits until
the pool times out.
Keep that connection for the inner session when the outer session is idle.
--
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]
