Am 27.06.2017 um 21:41 schrieb Rainer Jung:
Hi Violeta,

Am 16.06.2017 um 21:17 schrieb violet...@apache.org:
Author: violetagg
Date: Fri Jun 16 19:17:39 2017
New Revision: 1798977

URL: http://svn.apache.org/viewvc?rev=1798977&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61105
Add a new JULI FileHandler configuration for specifying the maximum
number of days to keep the log files. By default the log files will be
kept 90 days as configured in logging.properties.

Added:
    tomcat/trunk/test/org/apache/juli/TestFileHandler.java   (with props)
Modified:
    tomcat/trunk/conf/logging.properties
    tomcat/trunk/java/org/apache/juli/AsyncFileHandler.java
    tomcat/trunk/java/org/apache/juli/FileHandler.java
    tomcat/trunk/webapps/docs/changelog.xml
    tomcat/trunk/webapps/docs/logging.xml

...

Modified: tomcat/trunk/java/org/apache/juli/FileHandler.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/juli/FileHandler.java?rev=1798977&r1=1798976&r2=1798977&view=diff

==============================================================================

--- tomcat/trunk/java/org/apache/juli/FileHandler.java (original)
+++ tomcat/trunk/java/org/apache/juli/FileHandler.java Fri Jun 16
19:17:39 2017
...

@@ -74,24 +84,37 @@ import java.util.logging.LogRecord;
  *   <li><code>formatter</code> - The
<code>java.util.logging.Formatter</code>
  *    implementation class name for this Handler. Default value:
  *    <code>java.util.logging.SimpleFormatter</code></li>
+ *   <li><code>maxDays</code> - The maximum number of days to keep
the log
+ *    files. If the specified value is <code>&lt;=0</code> then the
log files
+ *    will be kept on the file system forever, otherwise they will be
kept the
+ *    specified maximum days. Default value: <code>-1</code>.</li>
  * </ul>
  */
 public class FileHandler extends Handler {
+    public static final int DEFAULT_MAX_DAYS = -1;
+
+    private static final ExecutorService DELETE_FILES_SERVICE =
Executors.newSingleThreadExecutor();

When testing the M22 release I noticed that a tomcat process was
leftover that didn't want to shut down. I checked and I could easily
reproduce by starting with startup.sh and then stopping with shutdown.sh
(but not using kill). IMHO it didn't shut down, because according to a
thread dump it had a single non-daemon thread still running (named
"pool-1-thread-1"). Since we typically give more specific names to all
threads we create I instrumented the Thread class to find out the
creator of that thread and noticed, that it was created here:

        at java.lang.Thread.<init>(Thread.java:677)
        at
java.util.concurrent.Executors$DefaultThreadFactory.newThread(Executors.java:613)

        at
java.util.concurrent.ThreadPoolExecutor$Worker.<init>(ThreadPoolExecutor.java:612)

        at
java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:925)

        at
java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1357)

        at
java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:112)

        at
java.util.concurrent.Executors$DelegatedExecutorService.submit(Executors.java:678)

        at org.apache.juli.FileHandler.clean(FileHandler.java:463)
        at org.apache.juli.FileHandler.<init>(FileHandler.java:117)
        at
org.apache.juli.AsyncFileHandler.<init>(AsyncFileHandler.java:82)
        at
org.apache.juli.AsyncFileHandler.<init>(AsyncFileHandler.java:74)

So it is the thread belonging to the above "ExecutorService
DELETE_FILES_SERVICE". I could not see, how that thread would ever get
stopped, so two remarks:

- in order to make sure TC can shut down without problem we either need
to stop that thread by ourselves during TC shutdown or mark it as a
daemon thread. I guess the latter is preferred.

- we should give the created thread a specific name according to its
typical task so that it can be identified in any thread dump. That
should be doable by the same ThreadFactory.

So we need to pass a ThreadFactory to the newSingleThreadExecutor() call
(this possibility already exists in the Executors class). To keep juli
independent from the other TC classes, we probably need to code the
factory inside juli, but we can borrow code from the small class
org.apache.tomcat.util.threads.TaskThreadFactory or use code from there
to extend the result of Executors.defaultThreadFactory() or
Executors.privilegedThreadFactory().

I should add: I only observed it for TC 9, because there the feature is active by default due to the updated juli config file, so the submit to the executor happens (attribute maxDays in logging.properties).

On all other branches the feature is off by default and although the executor gets created, I do not see the thread which probably means it gets created lazily during the first submit. That submit never happens with the default config, but would once someone actually tries to use the feature.

So of course we should fix on all branches and the feature is somewhat broken on all branches currently, not only on TC 9.

Regards,

Rainer


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

Reply via email to