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:30:09 UTC
[tomcat] branch main updated (aeb5b54 -> c2a56ec)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from aeb5b54 Rename
new f03933f Improve thread safety of open and close
new 6d84fe7 Alternative fix for PR #453 - lost log messages on shutdown
new c2a56ec 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 main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit f03933faf8e54ed2b75607ca47200ec2a410ec52
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 8176392..a218cc2 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() {
@@ -87,7 +88,12 @@ public class AsyncFileHandler extends FileHandler {
if (closed) {
return;
}
- closed = true;
+ synchronized (closeLock) {
+ if (closed) {
+ return;
+ }
+ closed = true;
+ }
super.close();
}
@@ -96,7 +102,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 main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit c2a56ec131db596265ccc9ffce3bbf34818f7c1a
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 d40cb1c..6a82bcb 100644
--- a/conf/catalina.policy
+++ b/conf/catalina.policy
@@ -88,7 +88,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 08a80ef..3613f7d 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);
@@ -206,10 +200,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 81bdf49..d7445f5 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -139,6 +139,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 8ec86a2..564fb87 100644
--- a/webapps/docs/config/systemprops.xml
+++ b/webapps/docs/config/systemprops.xml
@@ -189,13 +189,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 main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 6d84fe760ec58527719222614f9a7fbfea7e664a
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 a218cc2..08a80ef 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.
@@ -94,6 +95,7 @@ public class AsyncFileHandler extends FileHandler {
}
closed = true;
}
+ LoggerThread.deregisterHandler();
super.close();
}
@@ -108,6 +110,7 @@ public class AsyncFileHandler extends FileHandler {
}
closed = false;
}
+ LoggerThread.registerHandler();
super.open();
}
@@ -157,6 +160,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 ea01244..81bdf49 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -135,6 +135,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