You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2021/11/25 14:31:01 UTC

[tomcat] branch 9.0.x updated (25fe896 -> 1ac77ad)

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

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


    from 25fe896  Document conditions of use for AprLifecycleListener to avoid JVM crashes
     new dd849b1  Improve thread safety of open and close
     new a224cbd  Alternative fix for PR #453 - lost log messages on shutdown
     new 1ac77ad  Refactor AsyncFileHandler to make use of system property unnecessary

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 conf/catalina.policy                       |  1 -
 java/org/apache/juli/AsyncFileHandler.java | 67 ++++++++++++++++++++++++------
 webapps/docs/changelog.xml                 |  9 ++++
 webapps/docs/config/systemprops.xml        |  7 ----
 4 files changed, 64 insertions(+), 20 deletions(-)

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


[tomcat] 01/03: Improve thread safety of open and close

Posted by ma...@apache.org.
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

commit dd849b10bd85ee98755332479a04013afa969b3a
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Nov 25 13:50:42 2021 +0000

    Improve thread safety of open and close
---
 java/org/apache/juli/AsyncFileHandler.java | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/juli/AsyncFileHandler.java b/java/org/apache/juli/AsyncFileHandler.java
index f779d18..e947456 100644
--- a/java/org/apache/juli/AsyncFileHandler.java
+++ b/java/org/apache/juli/AsyncFileHandler.java
@@ -68,6 +68,7 @@ public class AsyncFileHandler extends FileHandler {
         logger.start();
     }
 
+    private final Object closeLock = new Object();
     protected volatile boolean closed = false;
 
     public AsyncFileHandler() {
@@ -88,7 +89,12 @@ public class AsyncFileHandler extends FileHandler {
         if (closed) {
             return;
         }
-        closed = true;
+        synchronized (closeLock) {
+            if (closed) {
+                return;
+            }
+            closed = true;
+        }
         super.close();
     }
 
@@ -97,7 +103,12 @@ public class AsyncFileHandler extends FileHandler {
         if (!closed) {
             return;
         }
-        closed = false;
+        synchronized (closeLock) {
+            if (!closed) {
+                return;
+            }
+            closed = false;
+        }
         super.open();
     }
 

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


[tomcat] 03/03: Refactor AsyncFileHandler to make use of system property unnecessary

Posted by ma...@apache.org.
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

commit 1ac77adb0f06d20f8c5691264daae29c000c31ce
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Nov 25 14:12:45 2021 +0000

    Refactor AsyncFileHandler to make use of system property unnecessary
    
    System property: org.apache.juli.AsyncLoggerPollInterval
---
 conf/catalina.policy                       |  1 -
 java/org/apache/juli/AsyncFileHandler.java | 12 ++----------
 webapps/docs/changelog.xml                 |  5 +++++
 webapps/docs/config/systemprops.xml        |  7 -------
 4 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/conf/catalina.policy b/conf/catalina.policy
index 91b7c83..7aab95d 100644
--- a/conf/catalina.policy
+++ b/conf/catalina.policy
@@ -89,7 +89,6 @@ grant codeBase "file:${catalina.home}/bin/tomcat-juli.jar" {
 
         permission java.util.PropertyPermission "java.util.logging.config.class", "read";
         permission java.util.PropertyPermission "java.util.logging.config.file", "read";
-        permission java.util.PropertyPermission "org.apache.juli.AsyncLoggerPollInterval", "read";
         permission java.util.PropertyPermission "org.apache.juli.AsyncMaxRecordCount", "read";
         permission java.util.PropertyPermission "org.apache.juli.AsyncOverflowDropType", "read";
         permission java.util.PropertyPermission "org.apache.juli.ClassLoaderLogManager.debug", "read";
diff --git a/java/org/apache/juli/AsyncFileHandler.java b/java/org/apache/juli/AsyncFileHandler.java
index e1ac89b..f76291c 100644
--- a/java/org/apache/juli/AsyncFileHandler.java
+++ b/java/org/apache/juli/AsyncFileHandler.java
@@ -33,8 +33,6 @@ import java.util.logging.LogRecord;
  *    Default value: <code>1</code></li>
  *   <li><code>org.apache.juli.AsyncMaxRecordCount</code>
  *    Default value: <code>10000</code></li>
- *   <li><code>org.apache.juli.AsyncLoggerPollInterval</code>
- *    Default value: <code>1000</code></li>
  * </ul>
  *
  * <p>See the System Properties page in the configuration reference of Tomcat.</p>
@@ -48,7 +46,6 @@ public class AsyncFileHandler extends FileHandler {
 
     public static final int DEFAULT_OVERFLOW_DROP_TYPE = 1;
     public static final int DEFAULT_MAX_RECORDS        = 10000;
-    public static final int DEFAULT_LOGGER_SLEEP_TIME  = 1000;
 
     public static final int OVERFLOW_DROP_TYPE = Integer.parseInt(
             System.getProperty("org.apache.juli.AsyncOverflowDropType",
@@ -56,9 +53,6 @@ public class AsyncFileHandler extends FileHandler {
     public static final int MAX_RECORDS = Integer.parseInt(
             System.getProperty("org.apache.juli.AsyncMaxRecordCount",
                                Integer.toString(DEFAULT_MAX_RECORDS)));
-    public static final int LOGGER_SLEEP_TIME = Integer.parseInt(
-            System.getProperty("org.apache.juli.AsyncLoggerPollInterval",
-                               Integer.toString(DEFAULT_LOGGER_SLEEP_TIME)));
 
     protected static final LinkedBlockingDeque<LogEntry> queue =
             new LinkedBlockingDeque<>(MAX_RECORDS);
@@ -207,10 +201,8 @@ public class AsyncFileHandler extends FileHandler {
         public void run() {
             while (true) {
                 try {
-                    LogEntry entry = queue.poll(LOGGER_SLEEP_TIME, TimeUnit.MILLISECONDS);
-                    if (entry != null) {
-                        entry.flush();
-                    }
+                    LogEntry entry = queue.take();
+                    entry.flush();
                 } catch (InterruptedException x) {
                     // Ignore the attempt to interrupt the thread.
                 } catch (Exception x) {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index cc1ff07..4d495d0 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -123,6 +123,11 @@
         Refactor the <code>AsyncFileHandler</code> to reduce the possibility of
         log messages being lost on shutdown. (markt)
       </fix>
+      <update>
+        Refactor the <code>AsyncFileHandler</code> to remove the need for the
+        <code>org.apache.juli.AsyncLoggerPollInterval</code>. If set, this
+        property now has no effect. (markt)
+      </update>
     </changelog>
   </subsection>
   <subsection name="Coyote">
diff --git a/webapps/docs/config/systemprops.xml b/webapps/docs/config/systemprops.xml
index c286703..2dfac10 100644
--- a/webapps/docs/config/systemprops.xml
+++ b/webapps/docs/config/systemprops.xml
@@ -463,13 +463,6 @@
       <p>The default value is <code>1</code> (drop the newest record in the queue).</p>
     </property>
 
-    <property name="org.apache.juli. AsyncLoggerPollInterval">
-      <p>The poll interval in milliseconds for the asynchronous logger thread.
-         If the log queue is empty, the async thread will issue a poll(poll interval)
-         in order to not wake up too often.</p>
-      <p>The default value is <code>1000</code> milliseconds.</p>
-    </property>
-
     <property name="org.apache.juli.logging. UserDataHelper.CONFIG">
       <p>The type of logging to use for errors generated by invalid input data.
          The options are: <code>DEBUG_ALL</code>, <code>INFO_THEN_DEBUG</code>,

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


[tomcat] 02/03: Alternative fix for PR #453 - lost log messages on shutdown

Posted by ma...@apache.org.
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

commit a224cbdb603c29a63c14a3f8b921b26870222f86
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Nov 25 14:04:55 2021 +0000

    Alternative fix for PR #453 - lost log messages on shutdown
---
 java/org/apache/juli/AsyncFileHandler.java | 40 ++++++++++++++++++++++++++++++
 webapps/docs/changelog.xml                 |  4 +++
 2 files changed, 44 insertions(+)

diff --git a/java/org/apache/juli/AsyncFileHandler.java b/java/org/apache/juli/AsyncFileHandler.java
index e947456..e1ac89b 100644
--- a/java/org/apache/juli/AsyncFileHandler.java
+++ b/java/org/apache/juli/AsyncFileHandler.java
@@ -18,6 +18,7 @@ package org.apache.juli;
 
 import java.util.concurrent.LinkedBlockingDeque;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.logging.LogRecord;
 /**
  * A {@link FileHandler} implementation that uses a queue of log entries.
@@ -95,6 +96,7 @@ public class AsyncFileHandler extends FileHandler {
             }
             closed = true;
         }
+        LoggerThread.deregisterHandler();
         super.close();
     }
 
@@ -109,6 +111,7 @@ public class AsyncFileHandler extends FileHandler {
             }
             closed = false;
         }
+        LoggerThread.registerHandler();
         super.open();
     }
 
@@ -158,6 +161,43 @@ public class AsyncFileHandler extends FileHandler {
     }
 
     protected static class LoggerThread extends Thread {
+
+        /*
+         * Implementation note: Use of this count could be extended to
+         * start/stop the LoggerThread but that would require careful locking as
+         * the current size of the queue also needs to be taken into account and
+         * there are lost of edge cases when rapidly starting and stopping
+         * handlers.
+         */
+        private static final AtomicInteger handlerCount = new AtomicInteger();
+
+        public static void registerHandler() {
+            handlerCount.incrementAndGet();
+        }
+
+        public static void deregisterHandler() {
+            int newCount = handlerCount.decrementAndGet();
+            if (newCount == 0) {
+                try {
+                    Thread dummyHook = new Thread();
+                    Runtime.getRuntime().addShutdownHook(dummyHook);
+                    Runtime.getRuntime().removeShutdownHook(dummyHook);
+                } catch (IllegalStateException ise) {
+                    // JVM is shutting down.
+                    // Allow up to 10s for for the queue to be emptied
+                    int sleepCount = 0;
+                    while (!AsyncFileHandler.queue.isEmpty() && sleepCount < 10000) {
+                        try {
+                            Thread.sleep(1);
+                        } catch (InterruptedException e) {
+                            // Ignore
+                        }
+                        sleepCount++;
+                    }
+                }
+            }
+        }
+
         public LoggerThread() {
             this.setDaemon(true);
             this.setName("AsyncFileHandlerWriter-" + System.identityHashCode(this));
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index c163c15..cc1ff07 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -119,6 +119,10 @@
         Document conditions under which the <code>AprLifecycleListener</code>
         can be used to avoid JVM crashes. (michaelo)
       </docs>
+      <fix>
+        Refactor the <code>AsyncFileHandler</code> to reduce the possibility of
+        log messages being lost on shutdown. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">

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