This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 36b2afffb30935b79fe6514ffcdf0bdd71093ca1 Author: opencode <[email protected]> AuthorDate: Wed Sep 30 23:04:04 2026 +0200 Capture and send the final session delta in DeltaSession.expire() before acquiring the session monitor to avoid a lock order inversion against the delta lock --- .../apache/catalina/ha/session/DeltaSession.java | 31 ++++++++++++++-------- webapps/docs/changelog.xml | 8 ++++++ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/java/org/apache/catalina/ha/session/DeltaSession.java b/java/org/apache/catalina/ha/session/DeltaSession.java index c13c3dc6e0..fdd6f96eb0 100644 --- a/java/org/apache/catalina/ha/session/DeltaSession.java +++ b/java/org/apache/catalina/ha/session/DeltaSession.java @@ -458,6 +458,26 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu return; } + /* + * Obtaining and sending the final delta below takes the delta lock. That must not happen while the session + * monitor is held: code that runs while the delta lock is held, in particular the listener notifications + * triggered by StandardSession's attribute methods, may enter methods that take the session monitor, and the + * two orders would deadlock the two threads. The delta is therefore captured and sent before synchronizing. + * Concurrent expirations of the same session may then duplicate this message, which receivers handle + * harmlessly (a delta for an unknown session is ignored). The expired notification below remains under the + * monitor so it is sent only once. + */ + String expiredId = getIdInternal(); + + if (notifyCluster && expiredId != null && manager instanceof DeltaManager) { + DeltaManager dmanager = (DeltaManager) manager; + CatalinaCluster cluster = dmanager.getCluster(); + ClusterMessage msg = dmanager.requestCompleted(expiredId, true); + if (msg != null) { + cluster.send(msg); + } + } + synchronized (this) { // Check again, now we are inside the sync so this code only runs once // Double check locking - isValid needs to be volatile @@ -469,17 +489,6 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu return; } - String expiredId = getIdInternal(); - - if (notifyCluster && expiredId != null && manager instanceof DeltaManager) { - DeltaManager dmanager = (DeltaManager) manager; - CatalinaCluster cluster = dmanager.getCluster(); - ClusterMessage msg = dmanager.requestCompleted(expiredId, true); - if (msg != null) { - cluster.send(msg); - } - } - super.expire(notify); if (notifyCluster) { diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index c4f0fdf116..5b77d4df65 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -188,6 +188,14 @@ other nodes like attributes set later on rather than remaining local to the node that started the application. (remm) </fix> + <fix> + Fix a possible deadlock in <code>DeltaSession.expire()</code>: the + final session delta is now captured and sent to the cluster before the + session monitor is acquired, because obtaining the delta takes the + delta lock and code that runs while the delta lock is held, such as + session attribute listener notifications, may in turn wait for the + session monitor. (remm) + </fix> </changelog> </subsection> <subsection name="Web applications"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
