This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 6ffae071705c4ef47c070a71ee5eff17ccd958aa 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 0549d599c7..c055f25ce3 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 cb15097a1f..4269be1ca1 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]
