aooohan commented on code in PR #545:
URL: https://github.com/apache/tomcat/pull/545#discussion_r950959440
##########
java/org/apache/juli/AsyncFileHandler.java:
##########
@@ -54,17 +60,12 @@ public class AsyncFileHandler extends FileHandler {
System.getProperty("org.apache.juli.AsyncMaxRecordCount",
Integer.toString(DEFAULT_MAX_RECORDS)));
- protected static final LinkedBlockingDeque<LogEntry> queue =
- new LinkedBlockingDeque<>(MAX_RECORDS);
-
- protected static final LoggerThread logger = new LoggerThread();
-
- static {
- logger.start();
- }
+ private static final ExecutorService LOGGER_SERVICE = new
LoggerExecutorService(OVERFLOW_DROP_TYPE,
+ DEFAULT_MAX_RECORDS);
Review Comment:
There should use `MAX_RECORDS`,not `DEFAULT_MAX_RECORDS`.
##########
java/org/apache/juli/AsyncFileHandler.java:
##########
@@ -117,115 +120,82 @@ public void publish(LogRecord record) {
// fill source entries, before we hand the record over to another
// thread with another class loader
record.getSourceMethodName();
- LogEntry entry = new LogEntry(record, this);
- boolean added = false;
- try {
- while (!added && !queue.offer(entry)) {
- switch (OVERFLOW_DROP_TYPE) {
- case OVERFLOW_DROP_LAST: {
- //remove the last added element
- queue.pollLast();
- break;
- }
- case OVERFLOW_DROP_FIRST: {
- //remove the first element in the queue
- queue.pollFirst();
- break;
- }
- case OVERFLOW_DROP_FLUSH: {
- added = queue.offer(entry, 1000,
TimeUnit.MILLISECONDS);
- break;
- }
- case OVERFLOW_DROP_CURRENT: {
- added = true;
- break;
- }
- }//switch
- }//while
- } catch (InterruptedException x) {
- // Allow thread to be interrupted and back out of the publish
- // operation. No further action required.
- }
+ loggerService.execute(new Runnable() {
+ @Override
+ public void run() {
+ if (!closed) {
+ publishInternal(record);
+ }
+ }
+ });
}
protected void publishInternal(LogRecord record) {
super.publish(record);
}
- 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();
+ static class LoggerExecutorService extends ThreadPoolExecutor {
+
+ private static final ThreadFactory THREAD_FACTORY = new
ThreadFactory(THREAD_PREFIX);
+
+ public LoggerExecutorService(final int overflowDropType, final int
maxRecords) {
+ super(1, 1, 0L, TimeUnit.MILLISECONDS, new
LinkedBlockingDeque<>(maxRecords), THREAD_FACTORY);
+ switch (overflowDropType) {
+ case OVERFLOW_DROP_LAST:
+ default:
+ setRejectedExecutionHandler(new DropLastPolicy());
+ break;
+ case OVERFLOW_DROP_FIRST:
+ setRejectedExecutionHandler(new DiscardOldestPolicy());
+ break;
+ case OVERFLOW_DROP_FLUSH:
+ setRejectedExecutionHandler(new DropFlushPolicy());
+ break;
+ case OVERFLOW_DROP_CURRENT:
+ setRejectedExecutionHandler(new DiscardPolicy());
+ }
}
- 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++;
- }
- }
- }
+ @Override
+ public LinkedBlockingDeque<Runnable> getQueue() {
+ return (LinkedBlockingDeque<Runnable>) super.getQueue();
}
- public LoggerThread() {
- this.setDaemon(true);
- this.setName("AsyncFileHandlerWriter-" +
System.identityHashCode(this));
+ @Override
+ public void finalize() {
+ shutdown();
}
+ }
+
+ private static class DropFlushPolicy implements RejectedExecutionHandler {
@Override
- public void run() {
+ public void rejectedExecution(Runnable r, ThreadPoolExecutor executor)
{
while (true) {
+ if (executor.isShutdown()) {
+ break;
+ }
try {
- LogEntry entry = queue.take();
- entry.flush();
- } catch (InterruptedException x) {
- // Ignore the attempt to interrupt the thread.
- } catch (Exception x) {
- x.printStackTrace();
+ if (executor.getQueue().offer(r, 1000,
TimeUnit.MILLISECONDS)) {
+ break;
+ }
+ ;
Review Comment:
This line needs to be removed.
##########
test/org/apache/juli/TestAsyncFileHandlerOverflow.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.juli;
+
+import static org.junit.Assert.assertEquals;
Review Comment:
Please do not use static import for the assertion tool as it is not very
clear.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
