This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new df82cbfd36 Fix incorrect cache size calculations for concurrent
PUT/DELETE
df82cbfd36 is described below
commit df82cbfd3610af439e97a2f6c2dbfb95d559bdad
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Oct 30 10:15:37 2024 +0000
Fix incorrect cache size calculations for concurrent PUT/DELETE
---
.../catalina/webresources/CachedResource.java | 27 ++++++++++++++++++----
webapps/docs/changelog.xml | 4 ++++
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/java/org/apache/catalina/webresources/CachedResource.java
b/java/org/apache/catalina/webresources/CachedResource.java
index 0b7543ce30..413ba477d8 100644
--- a/java/org/apache/catalina/webresources/CachedResource.java
+++ b/java/org/apache/catalina/webresources/CachedResource.java
@@ -73,6 +73,7 @@ public class CachedResource implements WebResource {
private volatile Boolean cachedExists = null;
private volatile Boolean cachedIsVirtual = null;
private volatile Long cachedContentLength = null;
+ private final Object cachedContentLengthLock = new Object();
public CachedResource(Cache cache, StandardRoot root, String path, long
ttl, int objectMaxSizeBytes,
@@ -243,13 +244,29 @@ public class CachedResource implements WebResource {
@Override
public long getContentLength() {
+ /*
+ * Cache the content length for two reasons.
+ *
+ * 1. It is relatively expensive to calculate, it shouldn't change and
this method is called multiple times
+ * during cache validation. Caching, therefore, offers a performance
benefit.
+ *
+ * 2. There is a race condition if concurrent threads are trying to
PUT and DELETE the same resource. If the
+ * DELETE thread removes the cache entry after the PUT thread has
created it but before validation is complete
+ * the DELETE thread sees a content length for a non-existant resource
but the PUT thread sees the actual
+ * content length. While that isn't an issue for the individual
requests it, does corrupt the cache size
+ * tracking as the size used for the resource is different between
when it is added to the cache and when it is
+ * removed. Caching combined with locking ensures that a consistent
content length is reported for the resource.
+ */
if (cachedContentLength == null) {
- long result = 0;
- if (webResource != null) {
- result = webResource.getContentLength();
- cachedContentLength = Long.valueOf(result);
+ synchronized (cachedContentLengthLock) {
+ if (cachedContentLength == null) {
+ if (webResource == null) {
+ cachedContentLength = Long.valueOf(0);
+ } else {
+ cachedContentLength =
Long.valueOf(webResource.getContentLength());
+ }
+ }
}
- return result;
}
return cachedContentLength.longValue();
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index bcee13ff64..7a72b65141 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -202,6 +202,10 @@
possible for subcomponents to be in <code>FAILED</code> after init.
(remm)
</fix>
+ <fix>
+ Fix incorrect cache size calculations when there is a concurrent PUT
and
+ DELETE for the same resource. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
