Propagate synchronous action failures in RollingFileManager and FileRenameAction (#1445, #1549)

Author: thisdudeiknew

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

@@ -45,7 +45,8 @@ public void testRename1() throws Exception {
 
         final File dest = new File(tempDir, "newFile.log");
         final FileRenameAction action = new FileRenameAction(file, dest, false);
-        action.execute();
+        boolean renameResult = action.execute();
+        assertTrue(renameResult, "Rename action returned false");
         assertTrue(dest.exists(), "Renamed file does not exist");
         assertFalse(file.exists(), "Old file exists");
     }
@@ -59,7 +60,8 @@ public void testEmpty() throws Exception {
         assertTrue(file.exists(), "File to rename does not exist");
         final File dest = new File(tempDir, "newFile.log");
         final FileRenameAction action = new FileRenameAction(file, dest, false);
-        action.execute();
+        boolean renameResult = action.execute();
+        assertTrue(renameResult, "Rename action returned false");
         assertFalse(dest.exists(), "Renamed file should not exist");
         assertFalse(file.exists(), "Old file still exists");
     }
@@ -73,7 +75,8 @@ public void testRenameEmpty() throws Exception {
         assertTrue(file.exists(), "File to rename does not exist");
         final File dest = new File(tempDir, "newFile.log");
         final FileRenameAction action = new FileRenameAction(file, dest, true);
-        action.execute();
+        boolean renameResult = action.execute();
+        assertTrue(renameResult, "Rename action returned false");
         assertTrue(dest.exists(), "Renamed file should exist");
         assertFalse(file.exists(), "Old file still exists");
     }
@@ -89,7 +92,8 @@ public void testNoParent() throws Exception {
 
         final File dest = new File(tempDir, "newFile.log");
         final FileRenameAction action = new FileRenameAction(file, dest, false);
-        action.execute();
+        boolean renameResult = action.execute();
+        assertTrue(renameResult, "Rename action returned false");
         assertTrue(dest.exists(), "Renamed file does not exist");
         assertFalse(file.exists(), "Old file exists");
     }

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

@@ -546,7 +546,7 @@ private boolean rollover(final RolloverStrategy strategy) {
                     asyncExecutor.execute(new AsyncAction(descriptor.getAsynchronous(), this));
                     releaseRequired = false;
                 }
-                return true;
+                return success;
             }
             return false;
         } finally {

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

@@ -191,7 +191,7 @@ public static boolean execute(final File source, final File destination, final b
             }
         } else {
             try {
-                source.delete();
+                return source.delete();
             } catch (final Exception exDelete) {
                 LOGGER.error(
                         "Unable to delete empty file {}: {} {}",

src/changelog/.3.x.x/1445_fix_synchronous_rolling_file_manager.xml

@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Licensed to the Apache Software Foundation (ASF) under one or more
+  ~ contributor license agreements.  See the NOTICE file distributed with
+  ~ this work for additional information regarding copyright ownership.
+  ~ The ASF licenses this file to you under the Apache License, Version 2.0
+  ~ (the "License"); you may not use this file except in compliance with
+  ~ the License.  You may obtain a copy of the License at
+  ~
+  ~      http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing, software
+  ~ distributed under the License is distributed on an "AS IS" BASIS,
+  ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  ~ See the License for the specific language governing permissions and
+  ~ limitations under the License.
+  -->
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns="http://logging.apache.org/log4j/changelog"
+       xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.1.xsd"
+       type="fixed">
+  <issue id="1445" link="https://github.com/apache/logging-log4j2/issues/1445"/>
+  <author id="thisdudeiknew" name="Timothy Pfeifer"/>
+  <description format="asciidoc">
+    Fixed `RollingFileManager` to propagate failed synchronous actions correctly.
+  </description>
+</entry>