ensure ordered writing of events in log files

Signed-off-by: Ceki Gulcu <[email protected]>
qos-ch · Dec 13, 2022 · 23301c2 · 23301c2
1 parent e62570b
commit 23301c2
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 37 deletions.
diff --git a/...ssic/src/test/java/ch/qos/logback/classic/rolling/TimeBasedRollingWithConfigFileTest.java b/...ssic/src/test/java/ch/qos/logback/classic/rolling/TimeBasedRollingWithConfigFileTest.java
@@ -147,7 +147,7 @@ public void timeAndSize() throws Exception {
         // match exactly the expected archive files. Thus, we aim for
         // an approximate match
         assertTrue(eCount >= 4 && eCount > expectedFilenameList.size() / 2,
-                "exitenceCount=" + eCount + ", expectedFilenameList.size=" + expectedFilenameList.size());
+                "existenceCount=" + eCount + ", expectedFilenameList.size=" + expectedFilenameList.size());
     }
 
     @Test

diff --git a/.../main/java/ch/qos/logback/core/rolling/DefaultTimeBasedFileNamingAndTriggeringPolicy.java b/.../main/java/ch/qos/logback/core/rolling/DefaultTimeBasedFileNamingAndTriggeringPolicy.java
@@ -1,15 +1,13 @@
 /**
- * Logback: the reliable, generic, fast and flexible logging framework.
- * Copyright (C) 1999-2015, QOS.ch. All rights reserved.
+ * Logback: the reliable, generic, fast and flexible logging framework. Copyright (C) 1999-2015, QOS.ch. All rights
+ * reserved.
  *
- * This program and the accompanying materials are dual-licensed under
- * either the terms of the Eclipse Public License v1.0 as published by
- * the Eclipse Foundation
+ * This program and the accompanying materials are dual-licensed under either the terms of the Eclipse Public License
+ * v1.0 as published by the Eclipse Foundation
  *
- *   or (per the licensee's choosing)
+ * or (per the licensee's choosing)
  *
- * under the terms of the GNU Lesser General Public License version 2.1
- * as published by the Free Software Foundation.
+ * under the terms of the GNU Lesser General Public License version 2.1 as published by the Free Software Foundation.
  */
 package ch.qos.logback.core.rolling;
 
@@ -21,9 +19,9 @@
 import ch.qos.logback.core.rolling.helper.TimeBasedArchiveRemover;
 
 /**
- * 
+ *
  * @author Ceki G&uuml;lc&uuml;
- * 
+ *
  * @param <E>
  */
 @NoAutoStart
@@ -49,16 +47,13 @@ public boolean isTriggeringEvent(File activeFile, final E event) {
         long currentTime = getCurrentTime();
         long localNextCheck = atomicNextCheck.get();
         if (currentTime >= localNextCheck) {
-            long nextCheckCandidate = computeNextCheck(currentTime);
-            boolean success = atomicNextCheck.compareAndSet(localNextCheck, nextCheckCandidate);
-            if(success) {
-                //Date dateOfElapsedPeriod = new Date(this.dateInCurrentPeriod.getTime());
-                Instant instantOfElapsedPeriod = dateInCurrentPeriod;
-                addInfo("Elapsed period: " + instantOfElapsedPeriod.toString());
-                this.elapsedPeriodsFileName = tbrp.fileNamePatternWithoutCompSuffix.convert(instantOfElapsedPeriod);
-                setDateInCurrentPeriod(currentTime);
-            }
-            return success;
+            long nextCheck = computeNextCheck(currentTime);
+            atomicNextCheck.set(nextCheck);
+            Instant instantOfElapsedPeriod = dateInCurrentPeriod;
+            addInfo("Elapsed period: " + instantOfElapsedPeriod.toString());
+            this.elapsedPeriodsFileName = tbrp.fileNamePatternWithoutCompSuffix.convert(instantOfElapsedPeriod);
+            setDateInCurrentPeriod(currentTime);
+            return true;
         } else {
             return false;
         }

diff --git a/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingFileAppender.java b/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingFileAppender.java
@@ -232,13 +232,21 @@ private void attemptRollover() {
      */
     @Override
     protected void subAppend(E event) {
+
+        // We need to synchronize on triggeringPolicy so that only one rollover
+        // occurs at a time. We should also ensure that the triggeringPolicy.isTriggeringEvent
+        // method can ensure that it updates itself properly when isTriggeringEvent returns true
+
         // The roll-over check must precede actual writing. This is the
         // only correct behavior for time driven triggers.
 
-        // the next method is assumed to return true only once per period (or whatever
-        // the decision criteria is) in a multi-threaded environment
-        if (triggeringPolicy.isTriggeringEvent(currentlyActiveFile, event)) {
-          rollover();
+        triggeringPolicyLock.lock();
+        try {
+            if (triggeringPolicy.isTriggeringEvent(currentlyActiveFile, event)) {
+                rollover();
+            }
+        } finally {
+            triggeringPolicyLock.unlock();
         }
 
 

diff --git a/logback-core/src/main/java/ch/qos/logback/core/rolling/SizeAndTimeBasedFNATP.java b/logback-core/src/main/java/ch/qos/logback/core/rolling/SizeAndTimeBasedFNATP.java
@@ -146,17 +146,15 @@ public boolean isTriggeringEvent(File activeFile, final E event) {
 
         // first check for roll-over based on time
         if (currentTime >= localNextCheck) {
-
             long nextCheckCandidate = computeNextCheck(currentTime);
-            boolean success = atomicNextCheck.compareAndSet(localNextCheck, nextCheckCandidate);
-            if (success) {
-                Instant instantInElapsedPeriod = dateInCurrentPeriod;
-                elapsedPeriodsFileName = tbrp.fileNamePatternWithoutCompSuffix.convertMultipleArguments(
-                        instantInElapsedPeriod, currentPeriodsCounter);
-                currentPeriodsCounter = 0;
-                setDateInCurrentPeriod(currentTime);
-            }
-            return success;
+            atomicNextCheck.set(nextCheckCandidate);
+            Instant instantInElapsedPeriod = dateInCurrentPeriod;
+            elapsedPeriodsFileName = tbrp.fileNamePatternWithoutCompSuffix.convertMultipleArguments(
+                    instantInElapsedPeriod, currentPeriodsCounter);
+            currentPeriodsCounter = 0;
+            setDateInCurrentPeriod(currentTime);
+
+            return true;
         }
 
         // next check for roll-over based on size

diff --git a/...ore/src/main/java/ch/qos/logback/core/rolling/TimeBasedFileNamingAndTriggeringPolicy.java b/...ore/src/main/java/ch/qos/logback/core/rolling/TimeBasedFileNamingAndTriggeringPolicy.java
@@ -1,6 +1,6 @@
 /**
  * Logback: the reliable, generic, fast and flexible logging framework.
- * Copyright (C) 1999-2015, QOS.ch. All rights reserved.
+ * Copyright (C) 1999-2022, QOS.ch. All rights reserved.
  *
  * This program and the accompanying materials are dual-licensed under
  * either the terms of the Eclipse Public License v1.0 as published by

diff --git a/logback-core/src/main/java/ch/qos/logback/core/rolling/TimeBasedRollingPolicy.java b/logback-core/src/main/java/ch/qos/logback/core/rolling/TimeBasedRollingPolicy.java
@@ -1,6 +1,6 @@
 /**
  * Logback: the reliable, generic, fast and flexible logging framework.
- * Copyright (C) 1999-2015, QOS.ch. All rights reserved.
+ * Copyright (C) 1999-2022, QOS.ch. All rights reserved.
  *
  * This program and the accompanying materials are dual-licensed under
  * either the terms of the Eclipse Public License v1.0 as published by