Issues with Poco::LogStream

[gregoire]

My wish is to be able to use a Poco::LogStream-like class in a multi-threaded context, and I had some trouble using Poco::LogStream directly.
I found an other solution that could replace the actual Poco::LogStream, at least for my special needs it works better.
The question is what do you think of my solution? Any comments to improve it or simply to point me out where I went wrong are welcome.

First I explain my needs and why I could not use Poco::LogStream, then I present my solution:

I have a myLog object that is a specific use of Poco Logging classes. I create a Logger, to log messages in a File and eventually on the Console.
I have one instance of this object in my application and several threads have access to it.
To be developper friendly I want to use a Poco::LogStream-like syntax.

If I use a shared instance of a Poco::LogStream

Code: Select all

class MyLog
{
   //...
   Poco::LogStream& operator()(); // return a reference to an intern instance of a Poco::LogStream
   //...
}

// Calling methods :
// thread 1 :
myLog() << "output " << i << " from thread 1" << std::endl;

// in the same time in thread 2 :
myLog() << "output " << i << " from thread 2" << std::endl;


I get undefined behaviour (loss of some character, duplicated messages,...), the order of the output cannot be predicted.
To use it I need one instance by calling thread. One solution would be: call a function that returns a new instance of a Poco::LogStream each time it is called.
So I tried something like that:

Code: Select all

class MyLog
{
   //...
   Poco::LogStream operator()(); // return a new Instance of Poco::LogStream to the calling thread
   //...
}

// Calling methods :
// thread 1 :
myLog() << "output " << i << " from thread 1" << std::endl;

// in the same time in thread 2 :
myLog() << "output " << i << " from thread 2" << std::endl;


Unfortunately I think I cannot define such a function,
because some things are private in the std, on which Poco::LogStream is highly based.

My Solution was creating a class with a template operator <<:

Code: Select all

#include <string>
#include <sstream>

#include <Poco/Logger.h>
#include <Poco/Message.h>

class StreamLogger
{
public:
    inline StreamLogger(Poco::Logger& logger)
        : _logger(logger)
        , _priority(Poco::Message::PRIO_INFORMATION)
        , oss ()
    {
    }
    inline StreamLogger(const StreamLogger& sl)
        : _logger(sl._logger)
        , _priority(sl._priority)
        , oss ()
    {
    }
    inline ~StreamLogger()
    {
      Poco::Message msg(_logger.name(), oss.str(), _priority);
      _logger.log(msg);
    }

    template<typename MessageType>
    inline StreamLogger& operator<<( const MessageType& s)
    {
        oss << s;
        return *this;
    }

    inline StreamLogger& operator<<( const Poco::Message::Priority& priority)
    {
        _priority = priority;
        return *this;
    }

private:
   Poco::Logger& _logger;
   Poco::Message::Priority _priority;
   std::ostringstream oss;
};

Then I can write:

Code: Select all

class MyLog
{
   //...
   Poco::StreamLogger operator()();
   //...
}


and safely do:

Code: Select all

// thread 1 :
myLog() << "output " << i << " from thread 1" << std::endl;

// in the same time in thread 2 :
myLog() << "output " << i << " from thread 2" << std::endl;


[alex]

I get undefined behaviour (loss of some character, duplicated messages,...), the order of the output cannot be predicted.

LogStream is inheriting from std::ostream, which is not synchronized. The messages streamed into LogStream are passed to the wrapped Logger (which in turn passes them to the log Channels) every time std::endl is streamed in. So, as you have already figured out, you have to synchronize access to the shared LogStream to avoid this behavior. That design, however, is inefficient and very susceptible to deadlocks because your wrapper will have to hold a lock until std::endl is streamed in (which may be a long time). Additionally, wrapping a wrapper does not make much sense - why reinvent the wheel - just give your users LogStream. It is more efficient to share the Logger wrapped in one LogStream per each thread - that way, the lock is only (and automatically - log channels are thread-safe) acquired when std::endl is streamed in and released after the message has been flushed through Logger to the Channel(s).

HTH