This is an automated email from the ASF dual-hosted git repository.
bneradt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new b13b9858ac Fix stderr closure bug in LogFile::check_fd() using fstat()
(#12407)
b13b9858ac is described below
commit b13b9858ac5b7e28b56bd267c25d86c09aac9bd0
Author: Jake Champion <[email protected]>
AuthorDate: Wed Sep 3 15:44:28 2025 +0100
Fix stderr closure bug in LogFile::check_fd() using fstat() (#12407)
The check_fd() function had a critical bug where it would close and reopen
log files to verify existence, which could inadvertently close stderr/stdout
when logging to these special files, breaking error reporting for the entire
process.
Fixed by replacing the unsafe access() + close/reopen pattern with fstat()
on the open file descriptor. This approach:
- Uses st_nlink == 0 to detect when regular files have been unlinked
- Never triggers for special files like stderr/stdout (they maintain
st_nlink > 0)
- Eliminates string comparisons and special-case handling
- Provides universal safety across all file descriptor types
- Maintains detection of externally moved/deleted log files for rotation
---
src/proxy/logging/LogFile.cc | 37 ++++++++++++++++++++------
tests/gold_tests/logging/log-filenames.test.py | 8 +-----
2 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/src/proxy/logging/LogFile.cc b/src/proxy/logging/LogFile.cc
index 86addcb66b..f7a7004c60 100644
--- a/src/proxy/logging/LogFile.cc
+++ b/src/proxy/logging/LogFile.cc
@@ -712,8 +712,8 @@ LogFile::writeln(char *data, int len, int fd, const char
*path)
LogFile::check_fd
This routine will occasionally stat the current logfile to make sure that
- it really does exist. The easiest way to do this is to close the file
- and re-open it, which will create the file if it doesn't already exist.
+ it really does exist. We use fstat() on the open file descriptor to check
+ if the file has been unlinked.
Failure to open the logfile will generate a manager alarm and a Warning.
-------------------------------------------------------------------------*/
@@ -726,13 +726,34 @@ LogFile::check_fd()
if ((stat_check_count % Log::config->file_stat_frequency) == 0) {
//
- // It's time to see if the file really exists. If we can't see
- // the file (via access), then we'll close our descriptor and
- // attempt to re-open it, which will create the file if it's not
- // there.
+ // Check if the file still exists using fstat() on the open file
descriptor.
+ // For regular files that have been unlinked (e.g., by log rotation tools),
+ // st_nlink will be 0. Non-regular files like stderr/stdout will never have
+ // st_nlink == 0, so this approach works safely for all file types.
//
- if (m_name && !LogFile::exists(m_name)) {
- close_file();
+ if (m_name && is_open()) {
+ int fd = get_fd();
+ if (fd >= 0) {
+ struct stat st;
+ if (fstat(fd, &st) == 0) {
+ // If the file has been unlinked, st_nlink will be 0
+ // This only happens for regular files, not special files like
stderr/stdout
+ if (st.st_nlink == 0) {
+ close_file();
+ }
+ } else {
+ // fstat failed, the file descriptor may be invalid
+ // Fall back to path-based existence check
+ if (!LogFile::exists(m_name)) {
+ close_file();
+ }
+ }
+ } else {
+ // No valid fd, fall back to path-based check
+ if (!LogFile::exists(m_name)) {
+ close_file();
+ }
+ }
}
stat_check_count = 0;
}
diff --git a/tests/gold_tests/logging/log-filenames.test.py
b/tests/gold_tests/logging/log-filenames.test.py
index a4fdbc73b3..5ec6d6d685 100644
--- a/tests/gold_tests/logging/log-filenames.test.py
+++ b/tests/gold_tests/logging/log-filenames.test.py
@@ -258,10 +258,4 @@ DefaultNamedTest()
CustomNamedTest()
stdoutTest()
-# The following stderr test can be run successfully by hand using the replay
-# files from the sandbox. All the expected output goes to stderr. However, for
-# some reason during the AuTest run, the stderr output stops emitting after the
-# logging.yaml file is parsed. This is left here for now because it is valuable
-# for use during development, but it is left commented out so that it doesn't
-# produce the false failure in CI and developer test runs.
-# stderrTest()
+stderrTest()
