(tomcat) branch 10.1.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 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


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

commit c0828a9de29bc38f31ee41414b05d420a8a0edbd
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 a19ebc1dde..4c179547d7 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