diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManagerTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManagerTest.java index 4a660861c20..325b859f6f1 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManagerTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManagerTest.java @@ -17,17 +17,21 @@ package org.apache.logging.log4j.core.appender.rolling; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStreamReader; +import java.io.OutputStream; import java.io.Reader; +import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import org.apache.logging.log4j.core.LoggerContext; import org.apache.logging.log4j.core.appender.RollingFileAppender; import org.apache.logging.log4j.core.appender.rolling.action.AbstractAction; @@ -138,9 +142,6 @@ public RolloverDescription rollover(final RollingFileManager manager) throws Sec assertNotNull(manager); manager.initialize(); - // Get the initialTime of this original log file - final long initialTime = manager.getFileTime(); - // Log something to ensure that the existing file size is > 0 final String testContent = "Test"; manager.writeToDestination(testContent.getBytes(StandardCharsets.US_ASCII), 0, testContent.length()); @@ -148,11 +149,11 @@ public RolloverDescription rollover(final RollingFileManager manager) throws Sec // Trigger rollover that will fail manager.rollover(); - // If the rollover fails, then the size should not be reset - assertNotEquals(0, manager.getFileSize()); + // If the rollover fails, then the log file should be unchanged + assertEquals(file.getAbsolutePath(), manager.getFileName()); - // The initialTime should not have changed - assertEquals(initialTime, manager.getFileTime()); + // The logged content should be unchanged + assertEquals(testContent, new String(Files.readAllBytes(file.toPath()), StandardCharsets.US_ASCII)); } @Test @@ -188,4 +189,39 @@ public void testCreateParentDir() { manager.close(); } } + + @Test + @Issue("https://github.com/apache/logging-log4j2/issues/2592") + public void testRolloverOfDeletedFile() throws IOException { + final File file = File.createTempFile("testRolloverOfDeletedFile", "log"); + file.deleteOnExit(); + final String testContent = "Test"; + try (final OutputStream os = + new ByteArrayOutputStream(); // use a dummy OutputStream so that the real file can be deleted + final RollingFileManager manager = new RollingFileManager( + null, + file.getAbsolutePath(), + "testRolloverOfDeletedFile.log.%d{yyyy-MM-dd}", + os, + true, + false, + 0, + System.currentTimeMillis(), + OnStartupTriggeringPolicy.createPolicy(1), + DefaultRolloverStrategy.newBuilder().build(), + file.getName(), + null, + null, + null, + null, + false, + ByteBuffer.allocate(256))) { + assertTrue(file.delete()); + manager.setRenameEmptyFiles(true); + manager.rollover(); + assertEquals(file.getAbsolutePath(), manager.getFileName()); + manager.writeBytes(testContent.getBytes(StandardCharsets.US_ASCII), 0, testContent.length()); + } + assertEquals(testContent, new String(Files.readAllBytes(file.toPath()), StandardCharsets.US_ASCII)); + } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java index a1030f3d045..b033f5785e0 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java @@ -604,43 +604,43 @@ public RolloverStrategy getRolloverStrategy() { private boolean rollover(final RolloverStrategy strategy) { - boolean releaseRequired = false; + boolean outputStreamClosed = false; try { // Block until the asynchronous operation is completed. semaphore.acquire(); - releaseRequired = true; } catch (final InterruptedException e) { logError("Thread interrupted while attempting to check rollover", e); - return false; + return outputStreamClosed; } - boolean success = true; + boolean asyncActionStarted = true; try { final RolloverDescription descriptor = strategy.rollover(this); if (descriptor != null) { writeFooter(); closeOutputStream(); + outputStreamClosed = true; + boolean syncActionSuccess = true; if (descriptor.getSynchronous() != null) { LOGGER.debug("RollingFileManager executing synchronous {}", descriptor.getSynchronous()); try { - success = descriptor.getSynchronous().execute(); + syncActionSuccess = descriptor.getSynchronous().execute(); } catch (final Exception ex) { - success = false; + syncActionSuccess = false; logError("Caught error in synchronous task", ex); } } - if (success && descriptor.getAsynchronous() != null) { + if (syncActionSuccess && descriptor.getAsynchronous() != null) { LOGGER.debug("RollingFileManager executing async {}", descriptor.getAsynchronous()); asyncExecutor.execute(new AsyncAction(descriptor.getAsynchronous(), this)); - releaseRequired = false; + asyncActionStarted = false; } - return success; } - return false; + return outputStreamClosed; } finally { - if (releaseRequired) { + if (asyncActionStarted) { semaphore.release(); } } diff --git a/src/changelog/.2.x.x/2592_fix_RollingFileManager_unsuccessful_rollover.xml b/src/changelog/.2.x.x/2592_fix_RollingFileManager_unsuccessful_rollover.xml new file mode 100644 index 00000000000..fb04758c251 --- /dev/null +++ b/src/changelog/.2.x.x/2592_fix_RollingFileManager_unsuccessful_rollover.xml @@ -0,0 +1,8 @@ + + + + Fix `RollingFileManager` to reopen the log file when the rollover was unsuccessful + \ No newline at end of file