Skip to content

Foundation: Synchronize channel replacement and prevent FileChannel data races #5509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Tyagiquamar wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Foundation: Synchronize channel replacement and prevent FileChannel data races #5509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions Foundation/include/Poco/FileChannel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ class Foundation_API FileChannel: public Channel

[[nodiscard]] RotateStrategy* createRotationStrategy(const std::string& rotation, const std::string& times) const;
[[nodiscard]] ArchiveStrategy* createArchiveStrategy(const std::string& archive, const std::string& times) const;
void unsafeOpen();

std::string _path;
std::string _times;
Expand Down
6 changes: 4 additions & 2 deletions Foundation/include/Poco/FormattingChannel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "Poco/Channel.h"
#include "Poco/Formatter.h"
#include "Poco/AutoPtr.h"
#include "Poco/Mutex.h"


namespace Poco {
Expand Down Expand Up @@ -89,8 +90,9 @@ class Foundation_API FormattingChannel: public Channel
~FormattingChannel() override;

private:
Formatter::Ptr _pFormatter;
Channel::Ptr _pChannel;
Formatter::Ptr _pFormatter;
Channel::Ptr _pChannel;
mutable FastMutex _mutex;
};


Expand Down
60 changes: 41 additions & 19 deletions Foundation/include/Poco/Logger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "Poco/Message.h"
#include "Poco/Format.h"
#include "Poco/AutoPtr.h"
#include "Poco/Mutex.h"
#include <map>
#include <vector>
#include <cstddef>
Expand Down Expand Up @@ -498,9 +499,10 @@ class Foundation_API Logger: public Channel
void logAlways(const std::string& text, Message::Priority prio);
void logAlways(std::string&& text, Message::Priority prio);

std::string _name;
Channel::Ptr _pChannel;
int _level;
std::string _name;
Channel::Ptr _pChannel;
int _level;
mutable FastMutex _channelMutex;

// definitions in Foundation.cpp
static LoggerMapPtr _pLoggerMap;
Expand Down Expand Up @@ -724,72 +726,92 @@ inline int Logger::getLevel() const

inline void Logger::log(const std::string& text, Message::Priority prio)
{
if (_level >= prio && _pChannel)
if (_level >= prio)
{
_pChannel->log(Message(_name, text, prio));
Channel::Ptr pChannel = getChannel();
if (pChannel)
{
pChannel->log(Message(_name, text, prio));
}
}
}


inline void Logger::log(std::string&& text, Message::Priority prio)
{
if (_level >= prio && _pChannel)
if (_level >= prio)
{
_pChannel->log(Message(_name, std::move(text), prio));
Channel::Ptr pChannel = getChannel();
if (pChannel)
{
pChannel->log(Message(_name, std::move(text), prio));
}
}
}


inline void Logger::logNPC(const std::string& text, Message::Priority prio)
{
if (_pChannel)
Channel::Ptr pChannel = getChannel();
if (pChannel)
{
_pChannel->log(Message(_name, text, prio));
pChannel->log(Message(_name, text, prio));
}
}


inline void Logger::logNPC(std::string&& text, Message::Priority prio)
{
if (_pChannel)
Channel::Ptr pChannel = getChannel();
if (pChannel)
{
_pChannel->log(Message(_name, std::move(text), prio));
pChannel->log(Message(_name, std::move(text), prio));
}
}


inline void Logger::log(const std::string& text, Message::Priority prio, const char* file, LineNumber line)
{
if (_level >= prio && _pChannel)
if (_level >= prio)
{
_pChannel->log(Message(_name, text, prio, file, line));
Channel::Ptr pChannel = getChannel();
if (pChannel)
{
pChannel->log(Message(_name, text, prio, file, line));
}
}
}


inline void Logger::log(std::string&& text, Message::Priority prio, const char* file, LineNumber line)
{
if (_level >= prio && _pChannel)
if (_level >= prio)
{
_pChannel->log(Message(_name, std::move(text), prio, file, line));
Channel::Ptr pChannel = getChannel();
if (pChannel)
{
pChannel->log(Message(_name, std::move(text), prio, file, line));
}
}
}


inline void Logger::logAlways(const std::string& text, Message::Priority prio)
{
if (_pChannel)
Channel::Ptr pChannel = getChannel();
if (pChannel)
{
_pChannel->log(Message(_name, text, prio));
pChannel->log(Message(_name, text, prio));
}
}


inline void Logger::logAlways(std::string&& text, Message::Priority prio)
{
if (_pChannel)
Channel::Ptr pChannel = getChannel();
if (pChannel)
{
_pChannel->log(Message(_name, std::move(text), prio));
pChannel->log(Message(_name, std::move(text), prio));
}
}

Expand Down
9 changes: 7 additions & 2 deletions Foundation/src/FileChannel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,12 @@ FileChannel::~FileChannel()
void FileChannel::open()
{
FastMutex::ScopedLock lock(_mutex);
unsafeOpen();
}


void FileChannel::unsafeOpen()
{
if (!_pFile)
{
_pFile = new LogFile(_path);
Expand Down Expand Up @@ -122,10 +127,10 @@ void FileChannel::close()

void FileChannel::log(const Message& msg)
{
open();

FastMutex::ScopedLock lock(_mutex);

unsafeOpen();

if (_pRotateStrategy->mustRotate(_pFile))
{
try
Expand Down
34 changes: 23 additions & 11 deletions Foundation/src/FormattingChannel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,41 +48,47 @@ FormattingChannel::~FormattingChannel()

void FormattingChannel::setFormatter(Formatter::Ptr pFormatter)
{
FastMutex::ScopedLock lock(_mutex);
_pFormatter = pFormatter;
}


Formatter::Ptr FormattingChannel::getFormatter() const
{
FastMutex::ScopedLock lock(_mutex);
return _pFormatter;
}


void FormattingChannel::setChannel(Channel::Ptr pChannel)
{
FastMutex::ScopedLock lock(_mutex);
_pChannel = pChannel;
}


Channel::Ptr FormattingChannel::getChannel() const
{
FastMutex::ScopedLock lock(_mutex);
return _pChannel;
}


void FormattingChannel::log(const Message& msg)
{
if (_pChannel)
Channel::Ptr pChannel = getChannel();
if (pChannel)
{
if (_pFormatter)
Formatter::Ptr pFormatter = getFormatter();
if (pFormatter)
{
std::string text;
_pFormatter->format(msg, text);
_pChannel->log(Message(msg, text));
pFormatter->format(msg, text);
pChannel->log(Message(msg, text));
}
else
{
_pChannel->log(msg);
pChannel->log(msg);
}
}
}
Expand All @@ -94,22 +100,28 @@ void FormattingChannel::setProperty(const std::string& name, const std::string&
setChannel(LoggingRegistry::defaultRegistry().channelForName(value));
else if (name == "formatter")
setFormatter(LoggingRegistry::defaultRegistry().formatterForName(value));
else if (_pChannel)
_pChannel->setProperty(name, value);
else
{
Channel::Ptr pChannel = getChannel();
if (pChannel)
pChannel->setProperty(name, value);
}
}


void FormattingChannel::open()
{
if (_pChannel)
_pChannel->open();
Channel::Ptr pChannel = getChannel();
if (pChannel)
pChannel->open();
}


void FormattingChannel::close()
{
if (_pChannel)
_pChannel->close();
Channel::Ptr pChannel = getChannel();
if (pChannel)
pChannel->close();
}


Expand Down
22 changes: 16 additions & 6 deletions Foundation/src/Logger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,14 @@ Logger::~Logger()

void Logger::setChannel(Channel::Ptr pChannel)
{
FastMutex::ScopedLock lock(_channelMutex);
_pChannel = pChannel;
}


Channel::Ptr Logger::getChannel() const
{
FastMutex::ScopedLock lock(_channelMutex);
return _pChannel;
}

Expand Down Expand Up @@ -77,9 +79,13 @@ void Logger::setProperty(const std::string& name, const std::string& value)
template <typename M>
void Logger::logImpl(M&& msg)
{
if (_level >= msg.getPriority() && _pChannel)
if (_level >= msg.getPriority())
{
_pChannel->log(std::forward<M>(msg));
Channel::Ptr pChannel = getChannel();
if (pChannel)
{
pChannel->log(std::forward<M>(msg));
}
}
}

Expand Down Expand Up @@ -110,11 +116,15 @@ void Logger::log(const Exception& exc, const char* file, LineNumber line)

void Logger::dump(const std::string& msg, const void* buffer, std::size_t length, Message::Priority prio)
{
if (_level >= prio && _pChannel)
if (_level >= prio)
{
std::string text(msg);
formatDump(text, buffer, length);
_pChannel->log(Message(_name, text, prio));
Channel::Ptr pChannel = getChannel();
if (pChannel)
{
std::string text(msg);
formatDump(text, buffer, length);
pChannel->log(Message(_name, text, prio));
}
}
}

Expand Down
Loading
Loading