Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions logback-core/src/main/java/ch/qos/logback/core/FileAppender.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.channels.FileChannel;
import java.nio.channels.FileLock;
import java.util.Map;
Expand Down Expand Up @@ -293,6 +294,20 @@ private void safeWriteBytes(byte[] byteArray) {
}
}

/*
* Returns the current position in the current file, as counted by
* {@link CountingOutputStream}, or zero if no file has been opened.
*/
protected long getCurrentFilePosition() {
OutputStream outputStream = getOutputStream();
if (outputStream == null) {
return 0;
} else {
// we already cast to a ResiliantFileOutputStream in #safeWrite()
return ((ResilientFileOutputStream) outputStream).getCount();
}
}

private void releaseFileLock(FileLock fileLock) {
if (fileLock != null && fileLock.isValid()) {
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package ch.qos.logback.core.recovery;

import java.io.IOException;
import java.io.OutputStream;

class CountingOutputStream extends OutputStream {

private final OutputStream delegate;

private long count;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be volatile. While there is external synchronization using various locks when calling write(), the new state of the variable may not be reflected immediately in other threads after this thread updates it. The increment operations += and ++ are actually each 2 operations, a read and then a write, this means the count may be incorrectly updated by another thread afterwards, as the value it reads could be stale.


CountingOutputStream(OutputStream delegate) {
this.delegate = delegate;
}

public long getCount() {
return count;
}

@Override
public void write(int b) throws IOException {
delegate.write(b);
count++;
}

@Override
public void write(byte[] b) throws IOException {
delegate.write(b);
count += b.length;
}

@Override
public void write(byte[] b, int off, int len) throws IOException {
delegate.write(b, off, len);
count += len;
}

@Override
public void flush() throws IOException {
delegate.flush();
}

@Override
public void close() throws IOException {
delegate.close();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,16 @@ public class ResilientFileOutputStream extends ResilientOutputStreamBase {

private File file;
private FileOutputStream fos;
private CountingOutputStream countingOutputStream;
private long originalFileLength;


public ResilientFileOutputStream(File file, boolean append, long bufferSize) throws FileNotFoundException {
this.file = file;
this.originalFileLength = append ? getFileLength(file) : 0;
fos = new FileOutputStream(file, append);
this.os = new BufferedOutputStream(fos, (int) bufferSize);
countingOutputStream = new CountingOutputStream(new BufferedOutputStream(fos, (int) bufferSize));
this.os = countingOutputStream;
this.presumedClean = true;
}

Expand All @@ -39,21 +44,35 @@ public File getFile() {
return file;
}

public long getCount() {
return originalFileLength + (countingOutputStream == null ? 0 : countingOutputStream.getCount());
}

@Override
String getDescription() {
return "file [" + file + "]";
}

@Override
OutputStream openNewOutputStream() throws IOException {
originalFileLength = getFileLength(file);
// see LOGBACK-765
fos = new FileOutputStream(file, true);
return new BufferedOutputStream(fos);
countingOutputStream = new CountingOutputStream(new BufferedOutputStream(fos));
return countingOutputStream;
}

@Override
public String toString() {
return "c.q.l.c.recovery.ResilientFileOutputStream@" + System.identityHashCode(this);
}

private static long getFileLength(File file) {
try {
return file.length();
} catch (Exception ignored) {
// file doesn't exist or we don't have permissions
return 0L;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ public void start() {
started = true;
}

public boolean isTriggeringEvent(final File activeFile, final E event, long currentFilePosition) {
return isTriggeringEvent(activeFile, event);
}

public boolean isTriggeringEvent(File activeFile, final E event) {
long currentTime = getCurrentTime();
long localNextCheck = atomicNextCheck.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ protected void subAppend(E event) {

triggeringPolicyLock.lock();
try {
if (triggeringPolicy.isTriggeringEvent(currentlyActiveFile, event)) {
if (triggeringPolicy.isTriggeringEvent(currentlyActiveFile, event, getCurrentFilePosition())) {
rollover();
}
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import java.io.File;
import java.time.Instant;
import java.util.Date;

import ch.qos.logback.core.CoreConstants;
import ch.qos.logback.core.joran.spi.NoAutoStart;
Expand All @@ -25,7 +24,6 @@
import ch.qos.logback.core.rolling.helper.SizeAndTimeBasedArchiveRemover;
import ch.qos.logback.core.util.Duration;
import ch.qos.logback.core.util.FileSize;
import ch.qos.logback.core.util.DefaultInvocationGate;
import ch.qos.logback.core.util.InvocationGate;
import ch.qos.logback.core.util.SimpleInvocationGate;

Expand Down Expand Up @@ -141,6 +139,11 @@ void computeCurrentPeriodsHighestCounterValue(final String stemRegex) {

@Override
public boolean isTriggeringEvent(File activeFile, final E event) {
return isTriggeringEvent(activeFile, event, UNKNOWN_FILE_POSITION);
}

@Override
public boolean isTriggeringEvent(File activeFile, final E event, long currentFilePosition) {

long currentTime = getCurrentTime();
long localNextCheck = atomicNextCheck.get();
Expand All @@ -151,17 +154,17 @@ public boolean isTriggeringEvent(File activeFile, final E event) {
atomicNextCheck.set(nextCheckCandidate);
Instant instantInElapsedPeriod = dateInCurrentPeriod;
elapsedPeriodsFileName = tbrp.fileNamePatternWithoutCompSuffix.convertMultipleArguments(
instantInElapsedPeriod, currentPeriodsCounter);
instantInElapsedPeriod, currentPeriodsCounter);
currentPeriodsCounter = 0;
setDateInCurrentPeriod(currentTime);

return true;
}

return checkSizeBasedTrigger(activeFile, currentTime);
return checkSizeBasedTrigger(activeFile, currentTime, currentFilePosition);
}

private boolean checkSizeBasedTrigger(File activeFile, long currentTime) {
private boolean checkSizeBasedTrigger(File activeFile, long currentTime, long currentFilePosition) {
// next check for roll-over based on size
if (invocationGate.isTooSoon(currentTime)) {
return false;
Expand All @@ -175,10 +178,11 @@ private boolean checkSizeBasedTrigger(File activeFile, long currentTime) {
addWarn("maxFileSize = null");
return false;
}
if (activeFile.length() >= maxFileSize.getSize()) {
long activeFileLength = currentFilePosition == UNKNOWN_FILE_POSITION ? activeFile.length() : currentFilePosition;
if (activeFileLength >= maxFileSize.getSize()) {

elapsedPeriodsFileName = tbrp.fileNamePatternWithoutCompSuffix.convertMultipleArguments(dateInCurrentPeriod,
currentPeriodsCounter);
currentPeriodsCounter);
currentPeriodsCounter++;
return true;
}
Expand All @@ -197,7 +201,7 @@ public void setCheckIncrement(Duration checkIncrement) {
@Override
public String getCurrentPeriodsFileNameWithoutCompressionSuffix() {
return tbrp.fileNamePatternWithoutCompSuffix.convertMultipleArguments(dateInCurrentPeriod,
currentPeriodsCounter);
currentPeriodsCounter);
}

public void setMaxFileSize(FileSize aMaxFileSize) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,17 @@ public void start() {
}


public boolean isTriggeringEvent(final File activeFile, final E event) {
@Override
public boolean isTriggeringEvent(File activeFile, E event, long currentFilePosition) {
long now = System.currentTimeMillis();
if (invocationGate.isTooSoon(now))
return false;
long activeFileLength = currentFilePosition >= 0 ? currentFilePosition : activeFile.length();
return (activeFileLength >= maxFileSize.getSize());
}

return (activeFile.length() >= maxFileSize.getSize());
public boolean isTriggeringEvent(final File activeFile, final E event) {
return isTriggeringEvent(activeFile, event, -1);
}

public FileSize getMaxFileSize() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ abstract public class TimeBasedFileNamingAndTriggeringPolicyBase<E> extends Cont
implements TimeBasedFileNamingAndTriggeringPolicy<E> {

static private String COLLIDING_DATE_FORMAT_URL = CODES_URL + "#rfa_collision_in_dateFormat";
protected static final int UNKNOWN_FILE_POSITION = -1;

protected TimeBasedRollingPolicy<E> tbrp;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@
* <code>TimeBasedRollingPolicy</code> is both easy to configure and quite
* powerful. It allows the rollover to be made based on time. It is possible to
* specify that the rollover occur once per day, per week or per month.
*
*
* <p>
* For more information, please refer to the online manual at
* http://logback.qos.ch/manual/appenders.html#TimeBasedRollingPolicy
*
*
* @author Ceki G&uuml;lc&uuml;
*/
public class TimeBasedRollingPolicy<E> extends RollingPolicyBase implements TriggeringPolicy<E> {
Expand Down Expand Up @@ -81,7 +81,7 @@ public void start() {

// wcs : without compression suffix
fileNamePatternWithoutCompSuffix = new FileNamePattern(
Compressor.computeFileNameStrWithoutCompSuffix(fileNamePatternStr, compressionMode), this.context);
Compressor.computeFileNameStrWithoutCompSuffix(fileNamePatternStr, compressionMode), this.context);

addInfo("Will use the pattern " + fileNamePatternWithoutCompSuffix + " for the active file");

Expand Down Expand Up @@ -152,7 +152,7 @@ private String transformFileNamePattern2ZipEntry(String fileNamePatternStr) {
}

public void setTimeBasedFileNamingAndTriggeringPolicy(
TimeBasedFileNamingAndTriggeringPolicy<E> timeBasedTriggering) {
TimeBasedFileNamingAndTriggeringPolicy<E> timeBasedTriggering) {
this.timeBasedFileNamingAndTriggeringPolicy = timeBasedTriggering;
}

Expand All @@ -173,11 +173,11 @@ public void rollover() throws RolloverFailure {
if (getParentsRawFileProperty() != null) {
renameUtil.rename(getParentsRawFileProperty(), elapsedPeriodsFileName);
} // else { nothing to do if CompressionMode == NONE and parentsRawFileProperty ==
// null }
// null }
} else {
if (getParentsRawFileProperty() == null) {
compressionFuture = compressor.asyncCompress(elapsedPeriodsFileName, elapsedPeriodsFileName,
elapsedPeriodStem);
elapsedPeriodStem);
} else {
compressionFuture = renameRawAndAsyncCompress(elapsedPeriodsFileName, elapsedPeriodStem);
}
Expand All @@ -197,26 +197,26 @@ Future<?> renameRawAndAsyncCompress(String nameOfCompressedFile, String innerEnt
}

/**
*
*
* The active log file is determined by the value of the parent's filename
* option. However, in case the file name is left blank, then, the active log
* file equals the file name for the current period as computed by the
* <b>FileNamePattern</b> option.
*
*
* <p>
* The RollingPolicy must know whether it is responsible for changing the name
* of the active file or not. If the active file name is set by the user via the
* configuration file, then the RollingPolicy must let it like it is. If the
* user does not specify an active file name, then the RollingPolicy generates
* one.
*
*
* <p>
* To be sure that the file name used by the parent class has been generated by
* the RollingPolicy and not specified by the user, we keep track of the last
* generated name object and compare its reference to the parent file name. If
* they match, then the RollingPolicy knows it's responsible for the change of
* the file name.
*
*
*/
public String getActiveFileName() {
String parentsRawFileProperty = getParentsRawFileProperty();
Expand All @@ -238,9 +238,22 @@ public boolean isTriggeringEvent(File activeFile, final E event) {
return timeBasedFileNamingAndTriggeringPolicy.isTriggeringEvent(activeFile, event);
}

/**
* Delegates to the underlying timeBasedFileNamingAndTriggeringPolicy.
*
* @param activeFile A reference to the currently active log file.
* @param event A reference to the current event.
* @param currentFilePosition The current position in activeFile, if known.
* @return
*/
@Override
public boolean isTriggeringEvent(File activeFile, final E event, long currentFilePosition) {
return timeBasedFileNamingAndTriggeringPolicy.isTriggeringEvent(activeFile, event, currentFilePosition);
}

/**
* Get the number of archive files to keep.
*
*
* @return number of archive files to keep
*/
public int getMaxHistory() {
Expand All @@ -249,7 +262,7 @@ public int getMaxHistory() {

/**
* Set the maximum number of archive files to keep.
*
*
* @param maxHistory number of archive files to keep
*/
public void setMaxHistory(int maxHistory) {
Expand All @@ -263,7 +276,7 @@ public boolean isCleanHistoryOnStart() {
/**
* Should archive removal be attempted on application start up? Default is
* false.
*
*
* @since 1.0.1
* @param cleanHistoryOnStart
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,6 @@ public interface TriggeringPolicy<E> extends LifeCycle {
* @return true if a roll-over should occur.
*/
boolean isTriggeringEvent(final File activeFile, final E event);

boolean isTriggeringEvent(final File activeFile, final E event, long currentFilePosition);
}
Loading