Resolve Github comments

Author: king-tyler

log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManagerTest.java

@@ -17,7 +17,6 @@
 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;
@@ -143,21 +142,18 @@ 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());
 
         // 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
@@ -197,14 +193,13 @@ public void testCreateParentDir() {
     @Test
     @Issue("https://github.com/apache/logging-log4j2/issues/2592")
     public void testRolloverOfDeletedFile() throws IOException {
-        final Configuration configuration = new NullConfiguration();
         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(
-                        configuration.getLoggerContext(),
+                        null,
                         file.getAbsolutePath(),
                         "testRolloverOfDeletedFile.log.%d{yyyy-MM-dd}",
                         os,
@@ -215,7 +210,7 @@ public void testRolloverOfDeletedFile() throws IOException {
                         OnStartupTriggeringPolicy.createPolicy(1),
                         DefaultRolloverStrategy.newBuilder().build(),
                         file.getName(),
-                        PatternLayout.createDefaultLayout(configuration),
+                        null,
                         null,
                         null,
                         null,

log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java

@@ -506,16 +506,9 @@ public synchronized void rollover() {
 
             if (rollover(rolloverStrategy)) {
                 try {
+                    size = 0;
+                    initialTime = System.currentTimeMillis();
                     createFileAfterRollover();
-                    final File file = new File(getFileName());
-                    size = file.length();
-                    try {
-                        final FileTime creationTime = (FileTime) Files.getAttribute(file.toPath(), "creationTime");
-                        initialTime = creationTime.toMillis();
-                    } catch (Exception ex) {
-                        LOGGER.warn("Unable to get current file time for {}", file);
-                        initialTime = System.currentTimeMillis();
-                    }
                 } catch (final IOException e) {
                     logError("Failed to create file after rollover", e);
                 }
@@ -611,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 true;
             }
-            return false;
+            return outputStreamClosed;
         } finally {
-            if (releaseRequired) {
+            if (asyncActionStarted) {
                 semaphore.release();
             }
         }