diff --git a/README.rst b/README.rst index d24ad4a34..7d9ba4515 100644 --- a/README.rst +++ b/README.rst @@ -710,7 +710,16 @@ To enable the log cleaner: .. code:: cpp - google::EnableLogCleaner(3); // keep your logs for 3 days + using namespace std::chrono_literals; + google::EnableLogCleaner(24h * 3); // keep your logs for 3 days + + +In C++20 (and later) this can be shortened to: + +.. code:: cpp + + using namespace std::chrono_literals; + google::EnableLogCleaner(3d); // keep your logs for 3 days And then glog will check if there are overdue logs whenever a flush is performed. In this example, any log file from your project whose last diff --git a/cmake/RunCleanerTest1.cmake b/cmake/RunCleanerTest1.cmake index 7fbf46336..9085e4844 100644 --- a/cmake/RunCleanerTest1.cmake +++ b/cmake/RunCleanerTest1.cmake @@ -7,16 +7,20 @@ foreach (iter RANGE 1 ${RUNS}) if (NOT _RESULT EQUAL 0) message (FATAL_ERROR "Failed to run logcleanup_unittest (error: ${_RESULT})") endif (NOT _RESULT EQUAL 0) - - # Ensure the log files to have different modification timestamps such that - # exactly one log file remains at the end. Otherwise all log files will be - # retained. - execute_process (COMMAND ${CMAKE_COMMAND} -E sleep 1) endforeach (iter) file (GLOB LOG_FILES ${TEST_DIR}/*.foobar) list (LENGTH LOG_FILES NUM_FILES) -if (NOT NUM_FILES EQUAL 1) - message (SEND_ERROR "Expected 1 log file in log directory but found ${NUM_FILES}") -endif (NOT NUM_FILES EQUAL 1) +if (WIN32) + # On Windows open files cannot be removed and will result in a permission + # denied error while unlinking such file. Therefore, the last file will be + # retained. + set (_expected 1) + else (WIN32) + set (_expected 0) +endif (WIN32) + +if (NOT NUM_FILES EQUAL _expected) + message (SEND_ERROR "Expected ${_expected} log file in log directory but found ${NUM_FILES}") +endif (NOT NUM_FILES EQUAL _expected) diff --git a/cmake/RunCleanerTest2.cmake b/cmake/RunCleanerTest2.cmake index d3b51e355..ad6988e5d 100644 --- a/cmake/RunCleanerTest2.cmake +++ b/cmake/RunCleanerTest2.cmake @@ -7,16 +7,20 @@ foreach (iter RANGE 1 ${RUNS}) if (NOT _RESULT EQUAL 0) message (FATAL_ERROR "Failed to run logcleanup_unittest (error: ${_RESULT})") endif (NOT _RESULT EQUAL 0) - - # Ensure the log files to have different modification timestamps such that - # exactly one log file remains at the end. Otherwise all log files will be - # retained. - execute_process (COMMAND ${CMAKE_COMMAND} -E sleep 1) endforeach (iter) file (GLOB LOG_FILES ${TEST_DIR}/test_cleanup_*.barfoo) list (LENGTH LOG_FILES NUM_FILES) -if (NOT NUM_FILES EQUAL 1) - message (SEND_ERROR "Expected 1 log file in build directory ${TEST_DIR} but found ${NUM_FILES}") -endif (NOT NUM_FILES EQUAL 1) +if (WIN32) + # On Windows open files cannot be removed and will result in a permission + # denied error while unlinking such file. Therefore, the last file will be + # retained. + set (_expected 1) + else (WIN32) + set (_expected 0) +endif (WIN32) + +if (NOT NUM_FILES EQUAL _expected) + message (SEND_ERROR "Expected ${_expected} log file in log directory but found ${NUM_FILES}") +endif (NOT NUM_FILES EQUAL _expected) diff --git a/cmake/RunCleanerTest3.cmake b/cmake/RunCleanerTest3.cmake index e8105d111..74721345a 100644 --- a/cmake/RunCleanerTest3.cmake +++ b/cmake/RunCleanerTest3.cmake @@ -10,19 +10,23 @@ foreach (iter RANGE 1 ${RUNS}) if (NOT _RESULT EQUAL 0) message (FATAL_ERROR "Failed to run logcleanup_unittest (error: ${_RESULT})") endif (NOT _RESULT EQUAL 0) - - # Ensure the log files to have different modification timestamps such that - # exactly one log file remains at the end. Otherwise all log files will be - # retained. - execute_process (COMMAND ${CMAKE_COMMAND} -E sleep 2) endforeach (iter) file (GLOB LOG_FILES ${TEST_DIR}/${TEST_SUBDIR}/test_cleanup_*.relativefoo) list (LENGTH LOG_FILES NUM_FILES) -if (NOT NUM_FILES EQUAL 1) - message (SEND_ERROR "Expected 1 log file in build directory ${TEST_DIR}${TEST_SUBDIR} but found ${NUM_FILES}") -endif (NOT NUM_FILES EQUAL 1) +if (WIN32) + # On Windows open files cannot be removed and will result in a permission + # denied error while unlinking such file. Therefore, the last file will be + # retained. + set (_expected 1) + else (WIN32) + set (_expected 0) +endif (WIN32) + +if (NOT NUM_FILES EQUAL _expected) + message (SEND_ERROR "Expected ${_expected} log file in build directory ${TEST_DIR}${TEST_SUBDIR} but found ${NUM_FILES}") +endif (NOT NUM_FILES EQUAL _expected) # Remove the subdirectory required by this unit test. file (REMOVE_RECURSE ${TEST_DIR}/${TEST_SUBDIR}) diff --git a/src/cleanup_immediately_unittest.cc b/src/cleanup_immediately_unittest.cc index 5804ab8d4..5d970064d 100644 --- a/src/cleanup_immediately_unittest.cc +++ b/src/cleanup_immediately_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2021, Google Inc. +// Copyright (c) 2024, Google Inc. // All rights reserved. // // Redistribution and use in source and binary forms, with or without @@ -55,8 +55,9 @@ using testing::StrNe; using namespace google; TEST(CleanImmediately, logging) { + using namespace std::chrono_literals; google::SetLogFilenameExtension(".foobar"); - google::EnableLogCleaner(0); + google::EnableLogCleaner(0h); for (unsigned i = 0; i < 1000; ++i) { LOG(INFO) << "cleanup test"; diff --git a/src/cleanup_with_absolute_prefix_unittest.cc b/src/cleanup_with_absolute_prefix_unittest.cc index 01b1ad296..8b9e243b3 100644 --- a/src/cleanup_with_absolute_prefix_unittest.cc +++ b/src/cleanup_with_absolute_prefix_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2021, Google Inc. +// Copyright (c) 2024, Google Inc. // All rights reserved. // // Redistribution and use in source and binary forms, with or without @@ -55,7 +55,8 @@ using testing::StrNe; using namespace google; TEST(CleanImmediatelyWithAbsolutePrefix, logging) { - google::EnableLogCleaner(0); + using namespace std::chrono_literals; + google::EnableLogCleaner(0h); google::SetLogFilenameExtension(".barfoo"); google::SetLogDestination(GLOG_INFO, "test_cleanup_"); diff --git a/src/cleanup_with_relative_prefix_unittest.cc b/src/cleanup_with_relative_prefix_unittest.cc index 9542e9fdd..3d9ef33b5 100644 --- a/src/cleanup_with_relative_prefix_unittest.cc +++ b/src/cleanup_with_relative_prefix_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2021, Google Inc. +// Copyright (c) 2024, Google Inc. // All rights reserved. // // Redistribution and use in source and binary forms, with or without @@ -55,7 +55,8 @@ using testing::StrNe; using namespace google; TEST(CleanImmediatelyWithRelativePrefix, logging) { - google::EnableLogCleaner(0); + using namespace std::chrono_literals; + google::EnableLogCleaner(0h); google::SetLogFilenameExtension(".relativefoo"); google::SetLogDestination(GLOG_INFO, "test_subdir/test_cleanup_"); diff --git a/src/glog/logging.h b/src/glog/logging.h index db87819d9..214af70c2 100644 --- a/src/glog/logging.h +++ b/src/glog/logging.h @@ -497,8 +497,12 @@ typedef void (*logging_fail_func_t)(); // Install a function which will be called after LOG(FATAL). GLOG_EXPORT void InstallFailureFunction(logging_fail_func_t fail_func); +[[deprecated( + "Use the type-safe std::chrono::minutes EnableLogCleaner overload " + "instead.")]] GLOG_EXPORT void +EnableLogCleaner(unsigned int overdue_days); // Enable/Disable old log cleaner. -GLOG_EXPORT void EnableLogCleaner(unsigned int overdue_days); +GLOG_EXPORT void EnableLogCleaner(const std::chrono::minutes& overdue); GLOG_EXPORT void DisableLogCleaner(); GLOG_EXPORT void SetApplicationFingerprint(const std::string& fingerprint); diff --git a/src/logging.cc b/src/logging.cc index 0bc961d4d..0ce8478f9 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -122,10 +123,6 @@ using std::perror; using std::fdopen; #endif -#ifdef _WIN32 -# define fdopen _fdopen -#endif - // There is no thread annotation support. #define EXCLUSIVE_LOCKS_REQUIRED(mu) @@ -346,13 +343,14 @@ static bool SendEmailInternal(const char* dest, const char* subject, base::Logger::~Logger() = default; namespace { + +constexpr std::intmax_t kSecondsInDay = 60 * 60 * 24; +constexpr std::intmax_t kSecondsInWeek = kSecondsInDay * 7; + // Optional user-configured callback to print custom prefixes. CustomPrefixCallback custom_prefix_callback = nullptr; // User-provided data to pass to the callback: void* custom_prefix_callback_data = nullptr; -} // namespace - -namespace { // Encapsulates all file-system related state class LogFileObject : public base::Logger { @@ -382,7 +380,7 @@ class LogFileObject : public base::Logger { // Internal flush routine. Exposed so that FlushLogFilesUnsafe() // can avoid grabbing a lock. Usually Flush() calls it after // acquiring lock_. - void FlushUnlocked(); + void FlushUnlocked(const std::chrono::system_clock::time_point& now); private: static const uint32 kRolloverAttemptFrequency = 0x20; @@ -413,31 +411,33 @@ class LogCleaner { public: LogCleaner(); - // Setting overdue_days to 0 days will delete all logs. - void Enable(unsigned int overdue_days); + // Setting overdue to 0 days will delete all logs. + void Enable(const std::chrono::minutes& overdue); void Disable(); - // update next_cleanup_time_ - void UpdateCleanUpTime(); - - void Run(bool base_filename_selected, const string& base_filename, + void Run(const std::chrono::system_clock::time_point& current_time, + bool base_filename_selected, const string& base_filename, const string& filename_extension); bool enabled() const { return enabled_; } private: - vector GetOverdueLogNames(string log_directory, unsigned int days, - const string& base_filename, - const string& filename_extension) const; + vector GetOverdueLogNames( + string log_directory, + const std::chrono::system_clock::time_point& current_time, + const string& base_filename, const string& filename_extension) const; bool IsLogFromCurrentProject(const string& filepath, const string& base_filename, const string& filename_extension) const; - bool IsLogLastModifiedOver(const string& filepath, unsigned int days) const; + bool IsLogLastModifiedOver( + const string& filepath, + const std::chrono::system_clock::time_point& current_time) const; bool enabled_{false}; - unsigned int overdue_days_{7}; + std::chrono::minutes overdue_{ + std::chrono::duration>{1}}; std::chrono::system_clock::time_point next_cleanup_time_; // cycle count at which to clean overdue log }; @@ -596,11 +596,12 @@ inline void LogDestination::FlushLogFilesUnsafe(int min_severity) { // about it std::for_each(std::next(std::begin(log_destinations_), min_severity), std::end(log_destinations_), - [](std::unique_ptr& log) { + [now = std::chrono::system_clock::now()]( + std::unique_ptr& log) { if (log != nullptr) { // Flush the base fileobject_ logger directly instead of // going through any wrappers to reduce chance of deadlock. - log->fileobject_.FlushUnlocked(); + log->fileobject_.FlushUnlocked(now); } }); } @@ -956,19 +957,19 @@ void LogFileObject::SetSymlinkBasename(const char* symlink_basename) { void LogFileObject::Flush() { std::lock_guard l{mutex_}; - FlushUnlocked(); + FlushUnlocked(std::chrono::system_clock::now()); } -void LogFileObject::FlushUnlocked() { +void LogFileObject::FlushUnlocked( + const std::chrono::system_clock::time_point& now) { if (file_ != nullptr) { fflush(file_); bytes_since_flush_ = 0; } // Figure out when we are due for another flush. next_flush_time_ = - std::chrono::system_clock::now() + - std::chrono::duration_cast( - std::chrono::duration{FLAGS_logbufsecs}); + now + std::chrono::duration_cast( + std::chrono::duration{FLAGS_logbufsecs}); } bool LogFileObject::CreateLogfile(const string& time_pid_string) { @@ -1085,6 +1086,18 @@ void LogFileObject::Write(bool force_flush, time_t timestamp, return; } + const auto now = std::chrono::system_clock::now(); + + auto cleanupLogs = [this, current_time = now] { + if (log_cleaner.enabled()) { + log_cleaner.Run(current_time, base_filename_selected_, base_filename_, + filename_extension_); + } + }; + + // Remove old logs + ScopedExit cleanupAtEnd{cleanupLogs}; + if (file_length_ >> 20U >= MaxLogSize() || PidHasChanged()) { if (file_ != nullptr) fclose(file_); file_ = nullptr; @@ -1195,7 +1208,7 @@ void LogFileObject::Write(bool force_flush, time_t timestamp, << "Running duration (h:mm:ss): " << PrettyDuration( std::chrono::duration_cast>( - std::chrono::system_clock::now() - start_time_)) + now - start_time_)) << '\n' << "Log line format: [IWEF]" << date_time_format << " " << "threadid file:line] msg" << '\n'; @@ -1226,7 +1239,7 @@ void LogFileObject::Write(bool force_flush, time_t timestamp, bytes_since_flush_ += message_len; } } else { - if (std::chrono::system_clock::now() >= next_flush_time_) { + if (now >= next_flush_time_) { stop_writing = false; // check to see if disk has free space. } return; // no need to flush @@ -1235,8 +1248,8 @@ void LogFileObject::Write(bool force_flush, time_t timestamp, // See important msgs *now*. Also, flush logs at least every 10^6 chars, // or every "FLAGS_logbufsecs" seconds. if (force_flush || (bytes_since_flush_ >= 1000000) || - (std::chrono::system_clock::now() >= next_flush_time_)) { - FlushUnlocked(); + (now >= next_flush_time_)) { + FlushUnlocked(now); #ifdef GLOG_OS_LINUX // Only consider files >= 3MiB if (FLAGS_drop_log_memory && file_length_ >= (3U << 20U)) { @@ -1259,41 +1272,33 @@ void LogFileObject::Write(bool force_flush, time_t timestamp, } } #endif - - // Remove old logs - if (log_cleaner.enabled()) { - log_cleaner.Run(base_filename_selected_, base_filename_, - filename_extension_); - } } } LogCleaner::LogCleaner() = default; -void LogCleaner::Enable(unsigned int overdue_days) { +void LogCleaner::Enable(const std::chrono::minutes& overdue) { enabled_ = true; - overdue_days_ = overdue_days; + overdue_ = overdue; } void LogCleaner::Disable() { enabled_ = false; } -void LogCleaner::UpdateCleanUpTime() { - next_cleanup_time_ = - std::chrono::system_clock::now() + - std::chrono::duration_cast( - std::chrono::duration{FLAGS_logcleansecs}); -} - -void LogCleaner::Run(bool base_filename_selected, const string& base_filename, +void LogCleaner::Run(const std::chrono::system_clock::time_point& current_time, + bool base_filename_selected, const string& base_filename, const string& filename_extension) { assert(enabled_); assert(!base_filename_selected || !base_filename.empty()); // avoid scanning logs too frequently - if (std::chrono::system_clock::now() < next_cleanup_time_) { + if (current_time < next_cleanup_time_) { return; } - UpdateCleanUpTime(); + + next_cleanup_time_ = + current_time + + std::chrono::duration_cast( + std::chrono::duration{FLAGS_logcleansecs}); vector dirs; @@ -1310,18 +1315,23 @@ void LogCleaner::Run(bool base_filename_selected, const string& base_filename, } } - for (auto& dir : dirs) { - vector logs = GetOverdueLogNames(dir, overdue_days_, base_filename, + for (const std::string& dir : dirs) { + vector logs = GetOverdueLogNames(dir, current_time, base_filename, filename_extension); - for (auto& log : logs) { - static_cast(unlink(log.c_str())); + for (const std::string& log : logs) { + // NOTE May fail on Windows if the file is still open + int result = unlink(log.c_str()); + if (result != 0) { + perror(("Could not remove overdue log " + log).c_str()); + } } } } vector LogCleaner::GetOverdueLogNames( - string log_directory, unsigned int days, const string& base_filename, - const string& filename_extension) const { + string log_directory, + const std::chrono::system_clock::time_point& current_time, + const string& base_filename, const string& filename_extension) const { // The names of overdue logs. vector overdue_log_names; @@ -1347,7 +1357,7 @@ vector LogCleaner::GetOverdueLogNames( if (IsLogFromCurrentProject(filepath, base_filename, filename_extension) && - IsLogLastModifiedOver(filepath, days)) { + IsLogLastModifiedOver(filepath, current_time)) { overdue_log_names.push_back(filepath); } } @@ -1444,16 +1454,17 @@ bool LogCleaner::IsLogFromCurrentProject( return true; } -bool LogCleaner::IsLogLastModifiedOver(const string& filepath, - unsigned int days) const { +bool LogCleaner::IsLogLastModifiedOver( + const string& filepath, + const std::chrono::system_clock::time_point& current_time) const { // Try to get the last modified time of this file. struct stat file_stat; if (stat(filepath.c_str(), &file_stat) == 0) { - const time_t seconds_in_a_day = 60 * 60 * 24; - time_t last_modified_time = file_stat.st_mtime; - time_t current_time = time(nullptr); - return difftime(current_time, last_modified_time) > days * seconds_in_a_day; + const auto last_modified_time = + std::chrono::system_clock::from_time_t(file_stat.st_mtime); + const auto diff = current_time - last_modified_time; + return diff >= overdue_; } // If failed to get file stat, don't return true! @@ -2627,7 +2638,13 @@ void ShutdownGoogleLogging() { } void EnableLogCleaner(unsigned int overdue_days) { - log_cleaner.Enable(overdue_days); + log_cleaner.Enable(std::chrono::duration_cast( + std::chrono::duration>{ + overdue_days})); +} + +void EnableLogCleaner(const std::chrono::minutes& overdue) { + log_cleaner.Enable(overdue); } void DisableLogCleaner() { log_cleaner.Disable(); } diff --git a/src/utilities.h b/src/utilities.h index fc272652f..7c985d2fd 100644 --- a/src/utilities.h +++ b/src/utilities.h @@ -1,4 +1,4 @@ -// Copyright (c) 2023, Google Inc. +// Copyright (c) 2024, Google Inc. // All rights reserved. // // Redistribution and use in source and binary forms, with or without @@ -28,6 +28,7 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. // // Author: Shinichiro Hamaji +// Sergiu Deitsch // // Define utilities for glog internal usage. @@ -54,6 +55,7 @@ #include #include +#include #include "glog/logging.h" @@ -193,6 +195,24 @@ void SetCrashReason(const CrashReason* r); void InitGoogleLoggingUtilities(const char* argv0); void ShutdownGoogleLoggingUtilities(); +template +class ScopedExit final { + public: + template ::value>* = nullptr> + constexpr explicit ScopedExit(F&& functor) noexcept( + std::is_nothrow_constructible::value) + : functor_{std::forward(functor)} {} + ~ScopedExit() noexcept(noexcept(std::declval()())) { functor_(); } + ScopedExit(const ScopedExit& other) = delete; + ScopedExit& operator=(const ScopedExit& other) = delete; + ScopedExit(ScopedExit&& other) noexcept = delete; + ScopedExit& operator=(ScopedExit&& other) noexcept = delete; + + private: + Functor functor_; +}; + } // namespace glog_internal_namespace_ } // namespace google