From cb702f72ef040e3974d3f25e228aaeb8971cbdb7 Mon Sep 17 00:00:00 2001 From: Maksim Kita Date: Mon, 5 Feb 2024 16:30:04 +0300 Subject: [PATCH] Updated implementation --- base/poco/Foundation/CMakeLists.txt | 6 - base/poco/Foundation/include/Poco/Logger.h | 7 +- .../include/Poco/RefCountedObject.h | 3 +- base/poco/Foundation/src/Logger.cpp | 112 +++++++----------- 4 files changed, 46 insertions(+), 82 deletions(-) diff --git a/base/poco/Foundation/CMakeLists.txt b/base/poco/Foundation/CMakeLists.txt index 5fe644d3057..dfb41a33fb1 100644 --- a/base/poco/Foundation/CMakeLists.txt +++ b/base/poco/Foundation/CMakeLists.txt @@ -166,12 +166,6 @@ set (SRCS ) add_library (_poco_foundation ${SRCS}) -target_link_libraries (_poco_foundation - PUBLIC - boost::headers_only - boost::system -) - add_library (Poco::Foundation ALIAS _poco_foundation) # TODO: remove these warning exclusions diff --git a/base/poco/Foundation/include/Poco/Logger.h b/base/poco/Foundation/include/Poco/Logger.h index 883294a071a..2a1cb33b407 100644 --- a/base/poco/Foundation/include/Poco/Logger.h +++ b/base/poco/Foundation/include/Poco/Logger.h @@ -23,8 +23,6 @@ #include #include -#include - #include "Poco/Channel.h" #include "Poco/Format.h" #include "Poco/Foundation.h" @@ -37,7 +35,7 @@ namespace Poco class Exception; class Logger; -using LoggerPtr = boost::intrusive_ptr; +using LoggerPtr = std::shared_ptr; class Foundation_API Logger : public Channel /// Logger is a special Channel that acts as the main @@ -953,9 +951,6 @@ private: static std::optional find(const std::string & name); static Logger * findRawPtr(const std::string & name); - friend void intrusive_ptr_add_ref(Logger * ptr); - friend void intrusive_ptr_release(Logger * ptr); - Logger(); Logger(const Logger &); Logger & operator=(const Logger &); diff --git a/base/poco/Foundation/include/Poco/RefCountedObject.h b/base/poco/Foundation/include/Poco/RefCountedObject.h index 1f806bdacb1..db966089e00 100644 --- a/base/poco/Foundation/include/Poco/RefCountedObject.h +++ b/base/poco/Foundation/include/Poco/RefCountedObject.h @@ -53,10 +53,11 @@ protected: virtual ~RefCountedObject(); /// Destroys the RefCountedObject. - mutable std::atomic _counter; private: RefCountedObject(const RefCountedObject &); RefCountedObject & operator=(const RefCountedObject &); + + mutable std::atomic _counter; }; diff --git a/base/poco/Foundation/src/Logger.cpp b/base/poco/Foundation/src/Logger.cpp index 16fc3a0480e..779af384b0b 100644 --- a/base/poco/Foundation/src/Logger.cpp +++ b/base/poco/Foundation/src/Logger.cpp @@ -302,9 +302,40 @@ void Logger::formatDump(std::string& message, const void* buffer, std::size_t le namespace { -inline LoggerPtr makeLoggerPtr(Logger & logger) +struct LoggerDeleter { - return LoggerPtr(&logger, false /*add_ref*/); + void operator()(Poco::Logger * logger) + { + std::lock_guard lock(getLoggerMutex()); + + /// If logger infrastructure is destroyed just decrement logger reference count + if (!_pLoggerMap) + { + logger->release(); + return; + } + + auto it = _pLoggerMap->find(logger->name()); + assert(it != _pLoggerMap->end()); + + /** If reference count is 1, this means this shared pointer owns logger + * and need destroy it. + */ + size_t reference_count_before_release = logger->release(); + if (reference_count_before_release == 1) + { + assert(it->second.owned_by_shared_ptr); + _pLoggerMap->erase(it); + } + } +}; + +inline LoggerPtr makeLoggerPtr(Logger & logger, bool owned_by_shared_ptr) +{ + if (owned_by_shared_ptr) + return LoggerPtr(&logger, LoggerDeleter()); + + return LoggerPtr(std::shared_ptr{}, &logger); } } @@ -327,15 +358,10 @@ LoggerPtr Logger::getShared(const std::string & name, bool should_be_owned_by_sh /** If during `unsafeGet` logger was created, then this shared pointer owns it. * If logger was already created, then this shared pointer does not own it. */ - if (inserted) - { - if (should_be_owned_by_shared_ptr_if_created) - it->second.owned_by_shared_ptr = true; - else - it->second.logger->duplicate(); - } + if (inserted && should_be_owned_by_shared_ptr_if_created) + it->second.owned_by_shared_ptr = true; - return makeLoggerPtr(*it->second.logger); + return makeLoggerPtr(*it->second.logger, it->second.owned_by_shared_ptr); } @@ -343,29 +369,20 @@ std::pair Logger::unsafeGet(const std::string& { std::optional optional_logger_it = find(name); - bool should_recreate_logger = false; - if (optional_logger_it) { auto & logger_it = *optional_logger_it; - std::optional reference_count_before; - if (get_shared) + if (logger_it->second.owned_by_shared_ptr) { - reference_count_before = logger_it->second.logger->duplicate(); - } - else if (logger_it->second.owned_by_shared_ptr) - { - reference_count_before = logger_it->second.logger->duplicate(); - logger_it->second.owned_by_shared_ptr = false; - } + logger_it->second.logger->duplicate(); - /// Other thread already decided to delete this logger, but did not yet remove it from map - if (reference_count_before && reference_count_before == 0) - should_recreate_logger = true; + if (!get_shared) + logger_it->second.owned_by_shared_ptr = false; + } } - if (!optional_logger_it || should_recreate_logger) + if (!optional_logger_it) { Logger * logger = nullptr; @@ -379,12 +396,6 @@ std::pair Logger::unsafeGet(const std::string& logger = new Logger(name, par.getChannel(), par.getLevel()); } - if (should_recreate_logger) - { - (*optional_logger_it)->second.logger = logger; - return std::make_pair(*optional_logger_it, true); - } - return add(logger); } @@ -412,7 +423,7 @@ LoggerPtr Logger::createShared(const std::string & name, Channel * pChannel, int auto [it, inserted] = unsafeCreate(name, pChannel, level); it->second.owned_by_shared_ptr = true; - return makeLoggerPtr(*it->second.logger); + return makeLoggerPtr(*it->second.logger, it->second.owned_by_shared_ptr); } Logger& Logger::root() @@ -479,43 +490,6 @@ Logger * Logger::findRawPtr(const std::string & name) } -void intrusive_ptr_add_ref(Logger * ptr) -{ - ptr->duplicate(); -} - - -void intrusive_ptr_release(Logger * ptr) -{ - size_t reference_count_before = ptr->_counter.fetch_sub(1, std::memory_order_acq_rel); - if (reference_count_before != 1) - return; - - { - std::lock_guard lock(getLoggerMutex()); - - if (_pLoggerMap) - { - auto it = _pLoggerMap->find(ptr->name()); - - /** It is possible that during release other thread created logger and - * updated iterator in map. - */ - if (it != _pLoggerMap->end() && ptr == it->second.logger) - { - /** If reference count is 0, this means this intrusive pointer owns logger - * and need destroy it. - */ - assert(it->second.owned_by_shared_ptr); - _pLoggerMap->erase(it); - } - } - } - - delete ptr; -} - - void Logger::names(std::vector& names) { std::lock_guard lock(getLoggerMutex());