diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/rolling/TimeBasedRollingWithConfigFileTest.java b/logback-classic/src/test/java/ch/qos/logback/classic/rolling/TimeBasedRollingWithConfigFileTest.java index bae59eed30..c02ed3e6ca 100755 --- a/logback-classic/src/test/java/ch/qos/logback/classic/rolling/TimeBasedRollingWithConfigFileTest.java +++ b/logback-classic/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/logback-core/src/main/java/ch/qos/logback/core/rolling/DefaultTimeBasedFileNamingAndTriggeringPolicy.java b/logback-core/src/main/java/ch/qos/logback/core/rolling/DefaultTimeBasedFileNamingAndTriggeringPolicy.java index 4071f6be5c..9e5289bc40 100644 --- a/logback-core/src/main/java/ch/qos/logback/core/rolling/DefaultTimeBasedFileNamingAndTriggeringPolicy.java +++ b/logback-core/src/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ülcü - * + * * @param */ @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 index daab58195a..7aeff97fe0 100755 --- 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 index af0ba5adb8..a9907ed85d 100755 --- 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/logback-core/src/main/java/ch/qos/logback/core/rolling/TimeBasedFileNamingAndTriggeringPolicy.java b/logback-core/src/main/java/ch/qos/logback/core/rolling/TimeBasedFileNamingAndTriggeringPolicy.java index 2e5e4b6c07..4fce1f9d9a 100644 --- a/logback-core/src/main/java/ch/qos/logback/core/rolling/TimeBasedFileNamingAndTriggeringPolicy.java +++ b/logback-core/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 index 98d86a0483..ce9a1d0467 100644 --- 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