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

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


The following commit(s) were added to refs/heads/8.5.x by this push:
     new 174d2a0810 Fix BZ 66535 - Redefine meaning of maxValidTime
174d2a0810 is described below

commit 174d2a0810ed1fe70f79d81a4cd9c6e313805d91
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Mar 23 11:28:37 2023 +0000

    Fix BZ 66535 - Redefine meaning of maxValidTime
    
    It is now the maximum time allowed between receiving parts of a
    transferred file. A new warning is logged if the transfer is cancelled.
---
 .../catalina/ha/deploy/FileMessageFactory.java     | 25 ++++++++++++++++++----
 .../catalina/ha/deploy/LocalStrings.properties     |  1 +
 webapps/docs/changelog.xml                         | 11 ++++++++++
 webapps/docs/config/cluster-deployer.xml           | 20 ++++++++++-------
 4 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/java/org/apache/catalina/ha/deploy/FileMessageFactory.java 
b/java/org/apache/catalina/ha/deploy/FileMessageFactory.java
index ba83be426a..4a4394af37 100644
--- a/java/org/apache/catalina/ha/deploy/FileMessageFactory.java
+++ b/java/org/apache/catalina/ha/deploy/FileMessageFactory.java
@@ -111,11 +111,20 @@ public class FileMessageFactory {
 
     /**
      * The time this instance was created. (in milliseconds)
+     *
+     * @deprecated Unused. This will be removed in Tomcat 11.
      */
+    @Deprecated
     protected long creationTime = 0;
 
+
+    /**
+     * The time this instance was last modified.
+     */
+    protected long lastModified = 0;
+
     /**
-     * The maximum valid time(in seconds) from creationTime.
+     * The maximum time (in seconds) this instance will be allowed to exist 
from lastModifiedTime.
      */
     protected int maxValidTime = -1;
 
@@ -150,6 +159,7 @@ public class FileMessageFactory {
             in = new FileInputStream(f);
         } // end if
         creationTime = System.currentTimeMillis();
+        lastModified = System.currentTimeMillis();
     }
 
     /**
@@ -231,6 +241,9 @@ public class FileMessageFactory {
             return false;
         }
 
+        // Have received a new message. Update the last modified time (even if 
the message is being buffered for now).
+        lastModified = System.currentTimeMillis();
+
         FileMessage next = null;
         synchronized (this) {
             if (!isWriting) {
@@ -350,11 +363,15 @@ public class FileMessageFactory {
     public boolean isValid() {
         if (maxValidTime > 0) {
             long timeNow = System.currentTimeMillis();
-            int timeIdle = (int) ((timeNow - creationTime) / 1000L);
+            long timeIdle = (timeNow - lastModified) / 1000L;
             if (timeIdle > maxValidTime) {
                 cleanup();
-                if (file.exists() && !file.delete()) {
-                    log.warn(sm.getString("fileMessageFactory.deleteFail", 
file));
+                if (file.exists()) {
+                    if (file.delete()) {
+                        log.warn(sm.getString("fileMessageFactory.delete", 
file, Long.toString(maxValidTime)));
+                    } else {
+                        log.warn(sm.getString("fileMessageFactory.deleteFail", 
file));
+                    }
                 }
                 return false;
             }
diff --git a/java/org/apache/catalina/ha/deploy/LocalStrings.properties 
b/java/org/apache/catalina/ha/deploy/LocalStrings.properties
index a7748595ac..57b98534d3 100644
--- a/java/org/apache/catalina/ha/deploy/LocalStrings.properties
+++ b/java/org/apache/catalina/ha/deploy/LocalStrings.properties
@@ -14,6 +14,7 @@
 # limitations under the License.
 
 farmWarDeployer.alreadyDeployed=webapp [{0}] are already deployed.
+farmWarDeployer.delete=Deleted [{0}] before the full file was received as the 
maxValidTime of [{1}] seconds has expired
 farmWarDeployer.deleteFail=Failed to delete [{0}]
 farmWarDeployer.deployEnd=Deployment from [{0}] finished.
 farmWarDeployer.fileCopyFail=Unable to copy from [{0}] to [{1}]
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 361b5a9b45..10ab299e6c 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -181,6 +181,17 @@
       </fix>
     </changelog>
   </subsection>
+  <subsection name="Cluster">
+    <changelog>
+      <fix>
+        <bug>66535</bug>: Redefine the <code>maxValidTime</code> attribute of
+        <code>FarmWarDeployer</code> to be the maximum time allowed between
+        receiving parts of a transferred file before the transfer is cancelled
+        and the associated resources cleaned-up. A new warning message will be
+        logged if the file transfer is cancelled. (markt)
+      </fix>
+    </changelog>
+  </subsection>
   <subsection name="WebSocket">
     <changelog>
       <fix>
diff --git a/webapps/docs/config/cluster-deployer.xml 
b/webapps/docs/config/cluster-deployer.xml
index 7b750f9d8c..f956e46fbc 100644
--- a/webapps/docs/config/cluster-deployer.xml
+++ b/webapps/docs/config/cluster-deployer.xml
@@ -84,14 +84,18 @@
         attribute will have no effect.
       </attribute>
       <attribute name="maxValidTime" required="false">
-        The maximum valid time(in seconds) of FileMessageFactory.
-        FileMessageFactory will be removed immediately after receiving the
-        complete WAR file but when failing to receive a FileMessage which was
-        sent dividing, FileMessageFactory will leak without being removed.
-        FileMessageFactory that is leaking will be automatically removed after
-        maxValidTime. If a negative value specified, FileMessageFactory will
-        never be removed. If the attribute is not provided, a default of 300
-        seconds (5 minutes) is used.
+        FileMessageFactory instances used by the FarmWarDeployer are  only
+        retained while they are required. When receiving a WAR file, the
+        associated FileMessageFactory instance is deleted once the WAR file has
+        been fully received. To avoid memory leaks under various error
+        conditions (part of the file never received, very slow message 
transfer,
+        etc.), this attribute defines the maximum time permitted between
+        receiving valid messages that contain part of the WAR file. If that
+        maximum time is exceeded, the FileMessageFactory will be deleted and 
the
+        WAR file transfer will fail for that node. If a negative value is
+        specified, the FileMessageFactory will only be removed once the WAR 
file
+        is fully recieved. If not specified, the default value of 300 (5
+        minutes) will be used.
       </attribute>
     </attributes>
 


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

Reply via email to