Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 29 additions & 8 deletions src/proxy/logging/LogFile.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
-------------------------------------------------------------------------*/
Expand All @@ -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();
}
}
}
Comment on lines +734 to 757
Copy link

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nested conditional structure creates excessive complexity with duplicated fallback logic. Consider extracting the fallback check into a helper function or restructuring the conditions to reduce nesting and eliminate the duplicate LogFile::exists(m_name) checks.

Copilot uses AI. Check for mistakes.
stat_check_count = 0;
}
Expand Down
8 changes: 1 addition & 7 deletions tests/gold_tests/logging/log-filenames.test.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,4 @@ def __init__(self):
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()