0

I have implemented a Logging class here.

class Log : public std::streambuf, public std::ostream {
  public:
    Log(int severity, const char* func, int line) :
        std::ostream(this), severity_(severity) {
        (this->severity_ == ERROR) ? std::cout << RED << "ERROR " :
                                     std::cout << RESET << "INFO ";
        std::cout << func << " @ line# " << line << ": ";
    }
    int overflow(int c) {
        if (this->severity_ == ERROR) {
            std::cout << RED;
            std::cout.put((char) c);
        } else {
            std::cout.put((char) c);
        }
        return 0;
    }
    ~Log() {
        std::cout << RESET << std::endl;
    }
    
  private:
    int severity_;
};

This works fine. But the output gets intermixed sometimes when two threads are executed concurrently. I went through several SO questions but they all discuss this in a broader way. I have this specific implementation which I want to fix. I tried adding a global mutex and calling this in a thread-safe way but the problem is that this file is a global header so cannot have a mutex variable defined globally here as it causes multiple definitions error. Is there any way to fix this?

Midhun
  • 744
  • 2
  • 15
  • 31
  • 1
    Global variables will get you in the end. Better would be a [static member variable](https://en.cppreference.com/w/cpp/language/static) so all the loggers share a logger-scoped mutex. – user4581301 Sep 11 '20 at 05:34
  • Side note: To solve the problem you had with the global mutex (which would likely be only the first of many) read [How do I use extern to share variables between source files?](https://stackoverflow.com/questions/1433204/how-do-i-use-extern-to-share-variables-between-source-files) – user4581301 Sep 11 '20 at 05:37
  • If you are using one `Log` object across multiple threads, the code using that object will be responsible for synchronising access to that object (creating a mutex, grabbing it before using the `Log`, releasing when done, etc). If you want multiple `Log` objects that all write to a common stream, the `Log` class needs to set up a single mutex (e.g. as a static member of the class), and all member functions need to grab the mutex before writing to the stream, and release the mutex after completing operations on the stream.The stream itself will probably also need to be a static class member. – Peter Sep 11 '20 at 06:25

1 Answers1

1

I worked on something like this some time ago. My solution was to add a member mutex (m_queue_mutex in the example) to control access to a member queue that stored strings to be logged, to which strings are enqueued using a function like

void print_log(std::string& msg)
{
    // Format the message here, maybe add some severity tag and line numbers
    // Lock the mutex, maybe something like
    //     std::lock_guard<mutex_type> queue_lock{m_queue_mutex};
    // Now enqueue the formatted string into the mutex
}

m_queue_mutex prevents simultaneous access to the logger queue, so also lock it when taking strings from the queue for output

My logger also had a thread that periodically locked the queue and output the contents to a file. This logging-thread approach had minimum impact on the threads that called the logger, because it removed the burden of i/o to another thread. I used it for a server daemon app requiring good performance.

DeltA
  • 564
  • 4
  • 12
  • Is the queue and m_queue_mutex global? Otherwise, there will be multiple instances and the result will be same. – Midhun Sep 11 '20 at 04:56
  • No, they are members. I will add clarification. The approach I mean allows the use of multiple logger objects, so no globals. – DeltA Sep 11 '20 at 04:56
  • I think I did not understand what you said before. If you want a global logger, just create a global object of your logger class, or maybe use it as a member of a singleton. And handle the logging of all threads as I posted in my answer (using a mutex or something). I thought you already had that part done. – DeltA Sep 11 '20 at 05:16