1
votes

I have a logging class in C++. It's a singleton with multithreading support.

What I wanted to achieve (and mainly did):

  • have a generic logger class
  • singleton
  • use it with stream insertion operator not as a function call

The class writes in a std::stringstream and on std::endl it flushes the std::stringstream content to file. A big drawback is that std::endl is required once and onlny once per call for everything to work correctly in a multithreading app. not sure if it's a good approach, but it's ok for now and in my case I can go with this drawback

My problem is that a call to std::setfill() when logging will deadlock. It seems that this call is not sent to my Logger class and is executed outside its critical section.

Can I achieve this? A logger class with full support for manips?

Log Calls

LOGI << "Some message " << aVariable << std::endl;                // OK
LOGI << std::setw(20) << var << std::endl                         // OK
LOGI << std::setfill('-') << std::setw(50) << var << std::endl;   // NOT OK because of setfill() call !!!!

Class Sample code

#define LOGFATAL Logger::GetLogger(LOGLEVEL_FATAL,      __FUNCTION__)
#define LOGE     Logger::GetLogger(LOGLEVEL_ERROR,      __FUNCTION__)
#define LOGW     Logger::GetLogger(LOGLEVEL_WARN,       __FUNCTION__)
#define LOGI     Logger::GetLogger(LOGLEVEL_INFO,       __FUNCTION__)
#define LOGD     Logger::GetLogger(LOGLEVEL_DEBUG,      __FUNCTION__)
#define LOGV     Logger::GetLogger(LOGLEVEL_VERBOSE,    __FUNCTION__)

class Logger
{
public:
    static void Initialize(const std::string& _logFile, const std::string& _logLevel)
    {
        isInitialized = true;
        logFileName = _logFile;
        logLevel = GetLogLevel(_logLevel);
    }
    static Logger& GetLogger(LogLevel l, const std::string& funcName)
    {
        if(isInitialized)
            csFile->lock();

        msgLogLevel = l;
        functionName = funcName;
        return GetLogger();
    }

    typedef std::ostream&  (*ManipFn)(std::ostream&);
    typedef std::ios_base& (*FlagsFn)(std::ios_base&);

    template<class T>
    Logger& operator<<(const T& output)
    {
        if (!isInitialized)
            return *this;

        m_stream << output;
        return *this;
    }

    Logger& operator<<(const ManipFn manip)
    {
        if (!isInitialized)
            return *this;

        manip(m_stream);

        if (manip == static_cast<ManipFn>(std::flush) ||
            manip == static_cast<ManipFn>(std::endl))
        {
            // flush the stream and unlock the mutex
            this->flush();
            csFile->unlock();
        }
        return *this;
    }

    Logger& operator<<(const FlagsFn manip) /// setiosflags, resetiosflags
    {
        manip(m_stream);
        return *this;
    }
...
    static Logger& GetLogger()
    {
        static Logger instance;
        return instance;
    }


Let me know if more information is needed

1
Some information is missing from your question. Where is Logger::Initialize called? Please show the definition of Logger::GetLogger() (is that where Initialize is called?). You state that the code has multithreading support but I don't see any synchronization primitives such as std::mutex etc.G.M.
In GetLogger() there is the csFile->lock() call and the unlock is done when std::endl is encountered. in Logger& operator<<(const ManipFn manip). csFile is std::recursive_mutex*. Logger::Initialize is called once in main(). Logger::GetLogger() is already in the sample code provided in my post.Alexandru Smeu
Where is the definition of Logger::GetLogger() in your code? The overload shown is GetLogger(LogLevel, const std::string&). Does that overload actually have default parameters? Sorry if I'm missing something.G.M.
Sorry... my bad ``` static Logger& GetLogger() /// instance can be got only if we pass a level { /// to GetLogger(LogLevel) method static Logger instance; return instance; } ```Alexandru Smeu
I added it also in the code sample from the post. I can't figure out how to post comments with formatting and line breaks (maybe it's not possible)Alexandru Smeu

1 Answers

0
votes

I missed my own words and my own code problems and limits (of which I was aware when I posted the question). My mistake was not to post the initial code which deadlocked my process. Here's the code:

LOGV << std::setfill('-') << std::setw(206) << '-' << std::endl;
LOGV << std::setfill(' ');

To quote myself "A big drawback is that std::endl is required once and onlny once per call for everything to work correctly in a multithreading app".

So the fix for my problem is LOGV << std::setfill('-') << std::setw(206) << '-' << std::setfill(' ') << std::endl;

Sorry for not checking in further, and for wasting anyone's time.