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>