jdaugherty commented on code in PR #16763:
URL: https://github.com/apache/grails-core/pull/16763#discussion_r4390857333
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/GrailsHibernateTemplate.java:
##########
@@ -220,6 +221,36 @@ 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();
Review Comment:
Reproduced. It is also a regression. This works on 8.0.x and fails on this
branch:
```groovy
WnsllAuthor.withTransaction { WnsllAuthor.count() } // disconnects the
request session
WnsllBook.withTransaction(propagationBehavior:
TransactionDefinition.PROPAGATION_SUPPORTS, readOnly: true) {
WnsllBook.withNewSession { WnsllAuthor.count() }
}
WnsllBook.count() // 8.0.x: 1. This branch: HibernateJdbcException,
connection is closed
```
It fails the same way with a `withTransaction` inside the `withNewSession`,
and with a read-write `SUPPORTS` scope.
The issue's own scenario is also still broken under `SUPPORTS`. The third
new feature, without its `assert WnsllBook.count() == 1` inside the scope,
fails after the scope with `PooledConnection has already been closed.` On 8.0.x
it threw the `NullPointerException`. That in-scope query registers the session
synchronization, which is why all three `SUPPORTS` features pass.
Registering a synchronization next to the `getAutoCommit()` call fixes all
of these. It runs before the `ConnectionSynchronization` and disconnects the
session, as `SpringSessionSynchronization.beforeCompletion` does. Because the
block now runs before the synchronizations are saved, the `finally` block
restores it with the others:
```java
if (txResources.isSynchronizationActive()) {
txResources.registerSynchronization(new TransactionSynchronization() {
@Override
public int getOrder() {
return SessionFactoryUtils.SESSION_SYNCHRONIZATION_ORDER;
}
@Override
public void beforeCompletion() {
if (logicalConnection.isOpen() &&
logicalConnection.isPhysicallyConnected()) {
logicalConnection.manualDisconnect();
}
}
});
}
```
With that, the cases above, `WithNewSessionLazyLoadSpec`, and
`GrailsHibernateTemplateSpec` pass.
--
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]
