From 535d510b487e6c5c6ab71da6bb338c54d713bb25 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Wed, 3 Jul 2013 19:07:59 -0700 Subject: [PATCH] Rewrite log file and log rotation behavior --- Builds/VisualStudio2012/RippleD.vcxproj | 7 ++ .../VisualStudio2012/RippleD.vcxproj.filters | 6 + modules/ripple_basics/ripple_basics.cpp | 1 + modules/ripple_basics/ripple_basics.h | 1 + modules/ripple_basics/utility/ripple_Log.cpp | 112 +++--------------- modules/ripple_basics/utility/ripple_Log.h | 23 ++-- .../ripple_basics/utility/ripple_LogFile.cpp | 69 +++++++++++ .../ripple_basics/utility/ripple_LogFile.h | 93 +++++++++++++++ .../ripple_core/functional/ripple_Config.h | 8 +- 9 files changed, 211 insertions(+), 109 deletions(-) create mode 100644 modules/ripple_basics/utility/ripple_LogFile.cpp create mode 100644 modules/ripple_basics/utility/ripple_LogFile.h diff --git a/Builds/VisualStudio2012/RippleD.vcxproj b/Builds/VisualStudio2012/RippleD.vcxproj index 85bbf2d707..59eb30c3de 100644 --- a/Builds/VisualStudio2012/RippleD.vcxproj +++ b/Builds/VisualStudio2012/RippleD.vcxproj @@ -88,6 +88,12 @@ true true + + true + true + true + true + true true @@ -1321,6 +1327,7 @@ + diff --git a/Builds/VisualStudio2012/RippleD.vcxproj.filters b/Builds/VisualStudio2012/RippleD.vcxproj.filters index fd67ab9a46..7c2bd2c131 100644 --- a/Builds/VisualStudio2012/RippleD.vcxproj.filters +++ b/Builds/VisualStudio2012/RippleD.vcxproj.filters @@ -822,6 +822,9 @@ [1] Ripple\ripple_app + + [1] Ripple\ripple_basics\utility + @@ -1533,6 +1536,9 @@ [1] Ripple\ripple_app\_main + + [1] Ripple\ripple_basics\utility + diff --git a/modules/ripple_basics/ripple_basics.cpp b/modules/ripple_basics/ripple_basics.cpp index 1fa03c5901..67495b7d51 100644 --- a/modules/ripple_basics/ripple_basics.cpp +++ b/modules/ripple_basics/ripple_basics.cpp @@ -56,6 +56,7 @@ namespace ripple #include "containers/ripple_TaggedCache.cpp" #include "utility/ripple_Log.cpp" +#include "utility/ripple_LogFile.cpp" #include "utility/ripple_ByteOrder.cpp" #include "utility/ripple_CountedObject.cpp" diff --git a/modules/ripple_basics/ripple_basics.h b/modules/ripple_basics/ripple_basics.h index 559e3d915d..34aa2d11d2 100644 --- a/modules/ripple_basics/ripple_basics.h +++ b/modules/ripple_basics/ripple_basics.h @@ -77,6 +77,7 @@ namespace ripple using namespace beast; +#include "utility/ripple_LogFile.h" #include "utility/ripple_Log.h" // Needed by others #include "types/ripple_BasicTypes.h" diff --git a/modules/ripple_basics/utility/ripple_Log.cpp b/modules/ripple_basics/utility/ripple_Log.cpp index 48eba348d3..348d252329 100644 --- a/modules/ripple_basics/utility/ripple_Log.cpp +++ b/modules/ripple_basics/utility/ripple_Log.cpp @@ -4,14 +4,10 @@ */ //============================================================================== -boost::recursive_mutex Log::sLock; - +LogFile Log::s_logFile; +boost::recursive_mutex Log::s_lock; LogSeverity Log::sMinSeverity = lsINFO; -std::ofstream* Log::outStream = NULL; -boost::filesystem::path* Log::pathToLog = NULL; -uint32 Log::logRotateCounter = 0; - //------------------------------------------------------------------------------ LogPartition* LogPartition::headLog = NULL; @@ -43,25 +39,6 @@ std::vector< std::pair > LogPartition::getSeverities ( //------------------------------------------------------------------------------ -// VFALCO TODO remove original code once we know the replacement is correct. -// Original code -/* -std::string ls = oss.str(); -size_t s = ls.find("\"secret\""); -if (s != std::string::npos) -{ - s += 8; - size_t sEnd = ls.size() - 1; - if (sEnd > (s + 35)) - sEnd = s + 35; - for (int i = s; i < sEnd; ++i) - ls[i] = '*'; -} -logMsg += ls; -*/ - -//------------------------------------------------------------------------------ - std::string Log::replaceFirstSecretWithAsterisks (std::string s) { using namespace std; @@ -144,14 +121,10 @@ Log::~Log () void Log::print (std::string const& text, bool toStdErr) { - boost::recursive_mutex::scoped_lock sl (sLock); + boost::recursive_mutex::scoped_lock sl (s_lock); - // Always write to the log file if it is open. - // - if (outStream != NULL) - { - (*outStream) << text << std::endl; - } + // Does nothing if not open. + s_logFile.writeln (text); if (toStdErr) { @@ -170,61 +143,23 @@ void Log::print (std::string const& text, bool toStdErr) } } -std::string Log::rotateLog (void) +std::string Log::rotateLog () { - boost::recursive_mutex::scoped_lock sl (sLock); - boost::filesystem::path abs_path; - std::string abs_path_str; + bool const wasOpened = s_logFile.closeAndReopen (); - uint32 failsafe = 0; - - std::string abs_new_path_str; - - do + if (wasOpened) { - std::string s; - std::stringstream out; - - failsafe++; - - if (failsafe == std::numeric_limits::max ()) - { - return "unable to create new log file; too many log files!"; - } - - abs_path = boost::filesystem::absolute (""); - abs_path /= *pathToLog; - abs_path_str = abs_path.parent_path ().string (); - - out << logRotateCounter; - s = out.str (); - - abs_new_path_str = abs_path_str + "/" + s + "_" + pathToLog->filename ().string (); - - logRotateCounter++; - + return "The log file was closed and reopened."; } - while (boost::filesystem::exists (boost::filesystem::path (abs_new_path_str))); - - outStream->close (); - - try + else { - boost::filesystem::rename (abs_path, boost::filesystem::path (abs_new_path_str)); + return "The log file could not be closed and reopened."; } - catch (...) - { - // unable to rename existing log file - } - - setLogFile (*pathToLog); - - return abs_new_path_str; } void Log::setMinSeverity (LogSeverity s, bool all) { - boost::recursive_mutex::scoped_lock sl (sLock); + boost::recursive_mutex::scoped_lock sl (s_lock); sMinSeverity = s; @@ -234,7 +169,7 @@ void Log::setMinSeverity (LogSeverity s, bool all) LogSeverity Log::getMinSeverity () { - boost::recursive_mutex::scoped_lock sl (sLock); + boost::recursive_mutex::scoped_lock sl (s_lock); return sMinSeverity; } @@ -292,28 +227,11 @@ LogSeverity Log::stringToSeverity (const std::string& s) void Log::setLogFile (boost::filesystem::path const& path) { - std::ofstream* newStream = new std::ofstream (path.c_str (), std::fstream::app); + bool const wasOpened = s_logFile.open (path.c_str ()); - if (!newStream->good ()) + if (! wasOpened) { Log (lsFATAL) << "Unable to open logfile " << path; - delete newStream; - newStream = NULL; - } - - boost::recursive_mutex::scoped_lock sl (sLock); - - if (outStream != NULL) - delete outStream; - - outStream = newStream; - - if (pathToLog != &path) - { - if (pathToLog != NULL) - delete pathToLog; - - pathToLog = new boost::filesystem::path (path); } } diff --git a/modules/ripple_basics/utility/ripple_Log.h b/modules/ripple_basics/utility/ripple_Log.h index 62f441c86e..13c44ed155 100644 --- a/modules/ripple_basics/utility/ripple_Log.h +++ b/modules/ripple_basics/utility/ripple_Log.h @@ -115,6 +115,13 @@ public: static void setLogFile (boost::filesystem::path const& pathToLogFile); + /** Rotate the log file. + + The log file is closed and reopened. This is for compatibility + with log management tools. + + @return A human readable string describing the result of the operation. + */ static std::string rotateLog (); public: @@ -185,18 +192,14 @@ private: maximumMessageCharacters = 12 * 1024 }; - // VFALCO TODO looks like there are really TWO classes in here. - // One is a stream target for '<<' operator and the other - // is a singleton. Split the singleton out to a new class. - // - static boost::recursive_mutex sLock; - static LogSeverity sMinSeverity; - static std::ofstream* outStream; - static boost::filesystem::path* pathToLog; - static uint32 logRotateCounter; - static std::string replaceFirstSecretWithAsterisks (std::string s); + // Singleton variables + // + static LogFile s_logFile; + static boost::recursive_mutex s_lock; + static LogSeverity sMinSeverity; + mutable std::ostringstream oss; LogSeverity mSeverity; std::string mPartitionName; diff --git a/modules/ripple_basics/utility/ripple_LogFile.cpp b/modules/ripple_basics/utility/ripple_LogFile.cpp new file mode 100644 index 0000000000..8c6d0900f8 --- /dev/null +++ b/modules/ripple_basics/utility/ripple_LogFile.cpp @@ -0,0 +1,69 @@ +//------------------------------------------------------------------------------ +/* + Copyright (c) 2011-2013, OpenCoin, Inc. +*/ +//============================================================================== + +LogFile::LogFile () + : m_stream (nullptr) +{ +} + +LogFile::~LogFile () +{ +} + +bool LogFile::isOpen () const noexcept +{ + return m_stream != nullptr; +} + +bool LogFile::open (boost::filesystem::path const& path) +{ + close (); + + bool wasOpened = false; + + // VFALCO TODO Make this work with Unicode file paths + ScopedPointer stream ( + new std::ofstream (path.c_str (), std::fstream::app)); + + if (stream->good ()) + { + m_path = path; + + m_stream = stream.release (); + + wasOpened = true; + } + + return wasOpened; +} + +bool LogFile::closeAndReopen () +{ + close (); + + return open (m_path); +} + +void LogFile::close () +{ + m_stream = nullptr; +} + +void LogFile::write (char const* text) +{ + if (m_stream != nullptr) + (*m_stream) << text; +} + +void LogFile::writeln (char const* text) +{ + if (m_stream != nullptr) + { + (*m_stream) << text; + (*m_stream) << std::endl; + } +} + diff --git a/modules/ripple_basics/utility/ripple_LogFile.h b/modules/ripple_basics/utility/ripple_LogFile.h new file mode 100644 index 0000000000..1d457a9ae6 --- /dev/null +++ b/modules/ripple_basics/utility/ripple_LogFile.h @@ -0,0 +1,93 @@ +//------------------------------------------------------------------------------ +/* + Copyright (c) 2011-2013, OpenCoin, Inc. +*/ +//============================================================================== + +#ifndef RIPPLE_LOGFILE_H_INCLUDED +#define RIPPLE_LOGFILE_H_INCLUDED + +/** Manages a system file containing logged output. + + The system file remains open during program execution. Interfaces + are provided for interoperating with standard log management + tools like logrotate(8): + + http://linuxcommand.org/man_pages/logrotate8.html + + @note None of the listed interfaces are thread-safe. +*/ +class LogFile : Uncopyable +{ +public: + /** Construct with no associated system file. + + A system file may be associated later with @ref open. + + @see open + */ + LogFile (); + + /** Destroy the object. + + If a system file is associated, it will be flushed and closed. + */ + ~LogFile (); + + /** Determine if a system file is associated with the log. + + @return `true` if a system file is associated and opened for writing. + */ + bool isOpen () const noexcept; + + /** Associate a system file with the log. + + If the file does not exist an attempt is made to create it + and open it for writing. If the file already exists an attempt is + made to open it for appending. + + If a system file is already associated with the log, it is closed first. + + @return `true` if the file was opened. + */ + // VFALCO NOTE The parameter is unfortunately a boost type because it + // can be either wchar or char based depending on platform. + // TODO Replace with beast::File + // + bool open (boost::filesystem::path const& path); + + /** Close and re-open the system file associated with the log + + This assists in interoperating with external log management tools. + + @return `true` if the file was opened. + */ + bool closeAndReopen (); + + /** Close the system file if it is open. + */ + void close (); + + /** write to the log file. + + Does nothing if there is no associated system file. + */ + void write (char const* text); + + /** write to the log file and append an end of line marker. + + Does nothing if there is no associated system file. + */ + void writeln (char const* text); + + /** Write to the log file using std::string. + */ + inline void write (std::string const& str) { write (str.c_str ()); } + inline void writeln (std::string const& str) { writeln (str.c_str ()); } + +private: + ScopedPointer m_stream; + boost::filesystem::path m_path; +}; + +#endif diff --git a/modules/ripple_core/functional/ripple_Config.h b/modules/ripple_core/functional/ripple_Config.h index d59da96ef6..8fee07eaa3 100644 --- a/modules/ripple_core/functional/ripple_Config.h +++ b/modules/ripple_core/functional/ripple_Config.h @@ -128,7 +128,7 @@ public: - The ledger is not advanced automatically. - If no ledger is loaded, the default ledger with the root account is created. - */ + */ bool RUN_STANDALONE; // Note: The following parameters do not relate to the UNL or trust at all @@ -201,7 +201,11 @@ public: */ String const getRpcAddress () { - return String (m_rpcIP.c_str ()) << ":" << m_rpcPort; + String s; + + s << m_rpcIP.c_str () << ":" << m_rpcPort; + + return s; } private: