This is an automated email from the ASF dual-hosted git repository.

pkarwasz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git


The following commit(s) were added to refs/heads/main by this push:
     new bc7807bb65 Propagate synchronous action failures in 
`RollingFileManager` and `FileRenameAction` (#1445, #1549)
bc7807bb65 is described below

commit bc7807bb6595c07c95c39280477ae0ca8b95d83f
Author: Tim P <[email protected]>
AuthorDate: Thu Oct 5 09:10:38 2023 -0400

    Propagate synchronous action failures in `RollingFileManager` and 
`FileRenameAction` (#1445, #1549)
---
 .../rolling/action/FileRenameActionTest.java       | 12 ++++++----
 .../core/appender/rolling/RollingFileManager.java  |  2 +-
 .../appender/rolling/action/FileRenameAction.java  |  2 +-
 .../1445_fix_synchronous_rolling_file_manager.xml  | 27 ++++++++++++++++++++++
 4 files changed, 37 insertions(+), 6 deletions(-)

diff --git 
a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameActionTest.java
 
b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameActionTest.java
index 32afc4b2a2..7309e6e7cc 100644
--- 
a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameActionTest.java
+++ 
b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameActionTest.java
@@ -45,7 +45,8 @@ public class FileRenameActionTest {
 
         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 class FileRenameActionTest {
         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 class FileRenameActionTest {
         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 class FileRenameActionTest {
 
         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");
     }
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 65e5a728c0..28d38eefc0 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
@@ -546,7 +546,7 @@ public class RollingFileManager extends FileManager {
                     asyncExecutor.execute(new 
AsyncAction(descriptor.getAsynchronous(), this));
                     releaseRequired = false;
                 }
-                return true;
+                return success;
             }
             return false;
         } finally {
diff --git 
a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java
 
b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java
index 7257811549..9fa6cdf57a 100644
--- 
a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java
+++ 
b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/FileRenameAction.java
@@ -191,7 +191,7 @@ public class FileRenameAction extends AbstractAction {
             }
         } else {
             try {
-                source.delete();
+                return source.delete();
             } catch (final Exception exDelete) {
                 LOGGER.error(
                         "Unable to delete empty file {}: {} {}",
diff --git a/src/changelog/.3.x.x/1445_fix_synchronous_rolling_file_manager.xml 
b/src/changelog/.3.x.x/1445_fix_synchronous_rolling_file_manager.xml
new file mode 100644
index 0000000000..4e446ebf50
--- /dev/null
+++ b/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>

Reply via email to