use setAndCompare instead of lock in RollingFileAppender.rollover, us…

…e Instant instead of Date in date related code

Signed-off-by: Ceki Gulcu <ceki@qos.ch>

logback-core/src/main/java/ch/qos/logback/core/rolling/DefaultTimeBasedFileNamingAndTriggeringPolicy.java

@@ -14,6 +14,7 @@
 package ch.qos.logback.core.rolling;
 
 import java.io.File;
+import java.time.Instant;
 import java.util.Date;
 
 import ch.qos.logback.core.joran.spi.NoAutoStart;
@@ -45,14 +46,19 @@ public void start() {
     }
 
     public boolean isTriggeringEvent(File activeFile, final E event) {
-        long time = getCurrentTime();
-        if (time >= nextCheck) {
-            Date dateOfElapsedPeriod = dateInCurrentPeriod;
-            addInfo("Elapsed period: " + dateOfElapsedPeriod);
-            elapsedPeriodsFileName = tbrp.fileNamePatternWithoutCompSuffix.convert(dateOfElapsedPeriod);
-            setDateInCurrentPeriod(time);
-            computeNextCheck();
-            return true;
+        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;
         } else {
             return false;
         }
@@ -62,4 +68,5 @@ public boolean isTriggeringEvent(File activeFile, final E event) {
     public String toString() {
         return "c.q.l.core.rolling.DefaultTimeBasedFileNamingAndTriggeringPolicy";
     }
+
 }

logback-core/src/main/java/ch/qos/logback/core/rolling/RollingFileAppender.java

@@ -20,6 +20,8 @@
 import java.io.IOException;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 
 import ch.qos.logback.core.CoreConstants;
 import ch.qos.logback.core.FileAppender;
@@ -43,6 +45,8 @@ public class RollingFileAppender<E> extends FileAppender<E> {
     TriggeringPolicy<E> triggeringPolicy;
     RollingPolicy rollingPolicy;
 
+    Lock triggeringPolicyLock = new ReentrantLock();
+
     static private String RFA_NO_TP_URL = CODES_URL + "#rfa_no_tp";
     static private String RFA_NO_RP_URL = CODES_URL + "#rfa_no_rp";
     static private String COLLISION_URL = CODES_URL + "#rfa_collision";
@@ -221,6 +225,8 @@ private void attemptRollover() {
         }
     }
 
+
+
     /**
      * This method differentiates RollingFileAppender from its super class.
      */
@@ -229,14 +235,13 @@ protected void subAppend(E event) {
         // The roll-over check must precede actual writing. This is the
         // only correct behavior for time driven triggers.
 
-        // We need to synchronize on triggeringPolicy so that only one rollover
-        // occurs at a time
-        synchronized (triggeringPolicy) {
-            if (triggeringPolicy.isTriggeringEvent(currentlyActiveFile, event)) {
-                rollover();
-            }
+        // 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();
         }
 
+
         super.subAppend(event);
     }
 

logback-core/src/main/java/ch/qos/logback/core/rolling/SizeAndTimeBasedFNATP.java

@@ -1,21 +1,20 @@
 /**
- * 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;
 
 import static ch.qos.logback.core.CoreConstants.MANUAL_URL_PREFIX;
 
 import java.io.File;
+import java.time.Instant;
 import java.util.Date;
 
 import ch.qos.logback.core.CoreConstants;
@@ -34,16 +33,15 @@ public class SizeAndTimeBasedFNATP<E> extends TimeBasedFileNamingAndTriggeringPo
 
     enum Usage {
         EMBEDDED, DIRECT
-    };
-
-    int currentPeriodsCounter = 0;
-    FileSize maxFileSize;
+    }
 
+    ;
 
+    volatile int currentPeriodsCounter = 0;
+    FileSize maxFileSize;
 
     Integer checkIncrement = null;
 
-    long nextSizeCheck = 0;
     static String MISSING_INT_TOKEN = "Missing integer token, that is %i, in FileNamePattern [";
     static String MISSING_DATE_TOKEN = "Missing date token, that is %d, in FileNamePattern [";
 
@@ -77,7 +75,7 @@ public void start() {
             withErrors();
         }
 
-        if(checkIncrement != null)
+        if (checkIncrement != null)
             invocationGate = new SimpleInvocationGate(checkIncrement);
 
         if (!validateDateAndIntegerTokens()) {
@@ -140,25 +138,29 @@ void computeCurrentPeriodsHighestCounterValue(final String stemRegex) {
         }
     }
 
-
     @Override
     public boolean isTriggeringEvent(File activeFile, final E event) {
 
-        long time = getCurrentTime();
+        long currentTime = getCurrentTime();
+        long localNextCheck = atomicNextCheck.get();
 
         // first check for roll-over based on time
-        if (time >= nextCheck) {
-            Date dateInElapsedPeriod = dateInCurrentPeriod;
-            elapsedPeriodsFileName = tbrp.fileNamePatternWithoutCompSuffix.convertMultipleArguments(dateInElapsedPeriod,
-                    currentPeriodsCounter);
-            currentPeriodsCounter = 0;
-            setDateInCurrentPeriod(time);
-            computeNextCheck();
-            return true;
+        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;
         }
 
         // next check for roll-over based on size
-        if (invocationGate.isTooSoon(time)) {
+        if (invocationGate.isTooSoon(currentTime)) {
             return false;
         }
 

logback-core/src/main/java/ch/qos/logback/core/rolling/TimeBasedFileNamingAndTriggeringPolicyBase.java

@@ -16,9 +16,11 @@
 import static ch.qos.logback.core.CoreConstants.CODES_URL;
 
 import java.io.File;
+import java.time.Instant;
 import java.util.Date;
 import java.util.Locale;
 import java.util.TimeZone;
+import java.util.concurrent.atomic.AtomicLong;
 
 import ch.qos.logback.core.CoreConstants;
 import ch.qos.logback.core.rolling.helper.ArchiveRemover;
@@ -38,9 +40,10 @@ abstract public class TimeBasedFileNamingAndTriggeringPolicyBase<E> extends Cont
     protected RollingCalendar rc;
 
     protected long artificialCurrentTime = -1;
-    protected Date dateInCurrentPeriod = null;
 
-    protected long nextCheck;
+    protected AtomicLong atomicNextCheck = new AtomicLong(0);
+    protected Instant dateInCurrentPeriod = null;
+
     protected boolean started = false;
     protected boolean errorFree = true;
 
@@ -73,33 +76,27 @@ public void start() {
             return;
         }
 
-        setDateInCurrentPeriod(new Date(getCurrentTime()));
+        long timestamp = getCurrentTime();
+        setDateInCurrentPeriod(timestamp);
+
         if (tbrp.getParentsRawFileProperty() != null) {
             File currentFile = new File(tbrp.getParentsRawFileProperty());
             if (currentFile.exists() && currentFile.canRead()) {
-                setDateInCurrentPeriod(new Date(currentFile.lastModified()));
+                timestamp = currentFile.lastModified();
+                setDateInCurrentPeriod(timestamp);
             }
         }
         addInfo("Setting initial period to " + dateInCurrentPeriod);
-        computeNextCheck();
+        long nextCheck = computeNextCheck(timestamp);
+        atomicNextCheck.set(nextCheck);
     }
 
     public void stop() {
         started = false;
     }
 
-    protected void computeNextCheck() {
-        nextCheck = rc.getNextTriggeringDate(dateInCurrentPeriod).getTime();
-    }
-
-    protected void setDateInCurrentPeriod(long now) {
-        dateInCurrentPeriod.setTime(now);
-    }
-
-    // allow Test classes to act on the dateInCurrentPeriod field to simulate old
-    // log files needing rollover
-    public void setDateInCurrentPeriod(Date _dateInCurrentPeriod) {
-        this.dateInCurrentPeriod = _dateInCurrentPeriod;
+    protected long computeNextCheck(long timestamp) {
+        return rc.getNextTriggeringDate(Instant.ofEpochMilli(timestamp)).toEpochMilli();
     }
 
     public String getElapsedPeriodsFileName() {
@@ -110,6 +107,10 @@ public String getCurrentPeriodsFileNameWithoutCompressionSuffix() {
         return tbrp.fileNamePatternWithoutCompSuffix.convert(dateInCurrentPeriod);
     }
 
+    protected void setDateInCurrentPeriod(long timestamp) {
+        dateInCurrentPeriod = Instant.ofEpochMilli(timestamp);
+    }
+
     public void setCurrentTime(long timeInMillis) {
         artificialCurrentTime = timeInMillis;
     }

logback-core/src/main/java/ch/qos/logback/core/rolling/TimeBasedRollingPolicy.java

@@ -17,6 +17,7 @@
 import static ch.qos.logback.core.CoreConstants.UNBOUNDED_TOTAL_SIZE_CAP;
 
 import java.io.File;
+import java.time.Instant;
 import java.util.Date;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
@@ -94,6 +95,7 @@ public void start() {
         }
         timeBasedFileNamingAndTriggeringPolicy.setContext(context);
         timeBasedFileNamingAndTriggeringPolicy.setTimeBasedRollingPolicy(this);
+        addInfo("xxxxxxxxxxxxxxxxxxx timeBasedFileNamingAndTriggeringPolicy  "+System.identityHashCode(timeBasedFileNamingAndTriggeringPolicy));
         timeBasedFileNamingAndTriggeringPolicy.start();
 
         if (!timeBasedFileNamingAndTriggeringPolicy.isStarted()) {
@@ -110,7 +112,7 @@ public void start() {
             archiveRemover.setTotalSizeCap(totalSizeCap.getSize());
             if (cleanHistoryOnStart) {
                 addInfo("Cleaning on start up");
-                Date now = new Date(timeBasedFileNamingAndTriggeringPolicy.getCurrentTime());
+                Instant now = Instant.ofEpochMilli(timeBasedFileNamingAndTriggeringPolicy.getCurrentTime());
                 cleanUpFuture = archiveRemover.cleanAsynchronously(now);
             }
         } else if (!isUnboundedTotalSizeCap()) {
@@ -183,7 +185,7 @@ public void rollover() throws RolloverFailure {
         }
 
         if (archiveRemover != null) {
-            Date now = new Date(timeBasedFileNamingAndTriggeringPolicy.getCurrentTime());
+            Instant now = Instant.ofEpochMilli(timeBasedFileNamingAndTriggeringPolicy.getCurrentTime());
             this.cleanUpFuture = archiveRemover.cleanAsynchronously(now);
         }
     }

logback-core/src/main/java/ch/qos/logback/core/rolling/helper/ArchiveRemover.java

@@ -13,6 +13,7 @@
  */
 package ch.qos.logback.core.rolling.helper;
 
+import java.time.Instant;
 import java.util.Date;
 import java.util.concurrent.Future;
 
@@ -24,11 +25,11 @@
  * @author Ceki Gülcü
  */
 public interface ArchiveRemover extends ContextAware {
-    void clean(Date now);
+    void clean(Instant instant);
 
     void setMaxHistory(int maxHistory);
 
     void setTotalSizeCap(long totalSizeCap);
 
-    Future<?> cleanAsynchronously(Date now);
+    Future<?> cleanAsynchronously(Instant now);
 }

logback-core/src/main/java/ch/qos/logback/core/rolling/helper/DateTokenConverter.java

@@ -13,6 +13,7 @@
  */
 package ch.qos.logback.core.rolling.helper;
 
+import java.time.Instant;
 import java.time.ZoneId;
 import java.util.Date;
 import java.util.List;
@@ -68,13 +69,21 @@ public String convert(Date date) {
         return cdf.format(date.getTime());
     }
 
+    public String convert(Instant instant) {
+        return cdf.format(instant.toEpochMilli());
+    }
+
     public String convert(Object o) {
         if (o == null) {
             throw new IllegalArgumentException("Null argument forbidden");
         }
         if (o instanceof Date) {
             return convert((Date) o);
         }
+        if (o instanceof Instant) {
+            return convert((Instant) o);
+        }
+
         throw new IllegalArgumentException("Cannot convert " + o + " of type" + o.getClass().getName());
     }
 
@@ -90,7 +99,11 @@ public ZoneId getZoneId() {
     }
 
     public boolean isApplicable(Object o) {
-        return (o instanceof Date);
+        if(o instanceof Date)
+            return true;
+        if(o instanceof Instant)
+            return true;
+        return false;
     }
 
     public String toRegex() {