(tomcat) branch 11.0.x updated: Re-work the fix for BZ 69527

[markt]

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/11.0.x by this push:
     new d4e12aee16 Re-work the fix for BZ 69527
d4e12aee16 is described below

commit d4e12aee1636bee4a4e48b3ffd0408f4049e3425
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Jan 14 07:22:20 2025 +0000

    Re-work the fix for BZ 69527
    
    Ensures that getSize() always returns the correct value which fixes both
    BZ 69527 and the original cache size corruption issue (the original fix
    for which caused BZ 69527(
---
 .../catalina/webresources/CachedResource.java      | 23 ++++++++++------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/java/org/apache/catalina/webresources/CachedResource.java 
b/java/org/apache/catalina/webresources/CachedResource.java
index 987cac39c0..c71807255d 100644
--- a/java/org/apache/catalina/webresources/CachedResource.java
+++ b/java/org/apache/catalina/webresources/CachedResource.java
@@ -75,7 +75,6 @@ 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();
     private volatile String cachedStrongETag = null;
 
 
@@ -248,28 +247,26 @@ public class CachedResource implements WebResource {
 
     @Override
     public long getContentLength() {
-        if (webResource == null) {
-            return 0;
-        }
         /*
          * 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.
+         * 2. There is a race condition if concurrent threads are trying to 
PUT and DELETE the same resource. See BZ
+         * 69527 (https://bz.apache.org/bugzilla/show_bug.cgi?id=69527#c14) 
for full details. The short version is that
+         * getContentLength() must always return the same value for any one 
CachedResource instance else the cache size
+         * will be corrupted.
          */
         if (cachedContentLength == null) {
-            synchronized (cachedContentLengthLock) {
-                if (cachedContentLength == null) {
-                    cachedContentLength = 
Long.valueOf(webResource.getContentLength());
+            if (webResource == null) {
+                synchronized (this) {
+                    if (webResource == null) {
+                        webResource = root.getResourceInternal(webAppPath, 
usesClassLoaderResources);
+                    }
                 }
             }
+            cachedContentLength = Long.valueOf(webResource.getContentLength());
         }
         return cachedContentLength.longValue();
     }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org