Adding Thread Safety to a Simple Logging Function?

Question

From what I read, the standard output streams are generally not thread safe. I have a C++ application (Windows-based, using Visual Studio 2005) that has a very simple logging function:

void logText(string text)
{
    if(g_OutputLogEnabled && g_OutputLog.is_open())
    {
        string logDate = getDateStamp("%Y-%m-%d %H:%M:%S");
        g_OutputLog << "[" << logDate << "]: " << text << endl;
    }

    cout << text << endl; // Also echo on stdout
}

In this example, g_OutputLog is an ofstream and g_OutputLogEnabled is a boolean.

I've been using this small function in my main application with no problem, but I now want to extend its use to some child threads. These threads do work and asynchronously print data as that work is done.

Question: How can I add simple, line-level thread-safety to this routine? All that I really care about is that each line that comes in remains intact in my log. Performance isn't a concern in this case, but (as always) faster is nicer.

I'm aware I could use third party logging packages, but I want to do it myself so I can learn how it works. My multi-threading knowledge isn't what it should be, and I'm trying to improve that.

I've heard the term critical sections, and I'm somewhat aware of mutexes and semaphores, but which would I use in this case? Is there a clean, simple solution? Thanks in advance for any advice.

Answer 1

Use scoped lock such as:

void logText(string text)
{
    if(g_OutputLogEnabled && g_OutputLog.is_open())
    {
        string logDate = getDateStamp("%Y-%m-%d %H:%M:%S");

        boost::scoped_lock (g_log_mutex);  //lock
        g_OutputLog << "[" << logDate << "]: " << text << endl;

    } //mutex is released automatically here
    
    boost::scoped_lock (g_cout_log_mutex); //lock on different mutex!
    cout << text << endl; // Also echo on stdout
}

Or you could use std::unique_lock if your compiler supports this.

---

How would you implement scoped_lock if you cannot use Boost and if you don't have std::unique_lock?

First define mutex class:

#include <Windows.h>

class mutex : private CRITICAL_SECTION  //inherit privately!
{
public:
     mutex() 
     {
        ::InitializeCriticalSection(this);
     }
     ~mutex() 
     {
        ::DeleteCriticalSection(this);
     }
private:

     friend class scoped_lock;  //make scoped_lock a friend of mutex

     //disable copy-semantic 
     mutex(mutex const &);           //do not define it!
     void operator=(mutex const &);  //do not define it!

     void lock() 
     {
        ::EnterCriticalSection(this);
     }
     void unlock() 
     {
        ::LeaveCriticalSection(this);
     }
};

Then define scoped_lock as:

class scoped_lock
{
      mutex & m_mutex;
  public:
      scoped_lock(mutex & m) : m_mutex(m) 
      {
          m_mutex.lock();
      }
      ~scoped_lock()
      {
          m_mutex.unlock();
      }
};

Now you can use them.

Answer 2

Given that the function is clearly not performance oriented, add a simple mutex and lock before writing to the streams.

Of course for safety reasons the lock should be automatically released, so be sure to use RAII.

I would recommend a look at the Boost.Threads library.

Answer 3

Yes, you should add some kind of protection to your code. You don't need anything exotic, you want to access a resource (your stream) while it may be in use by something else. Two types of synchronization objects perform well with this task: Critical Sections and Mutex. For more details about locks you may start reading this article on Wikipedia. Mutex usually are slower but can be shared across processes, this is not your case so you may use a simple Critical Section to synchronize your threads.

Few tips if you do not plan to use a 3rd part library (like the great Boost).

When EnterCriticalSection function to acquire the lock. If the resource is locked by someone else your thread will be suspended and reactivated when the resource will be released by its owner (moreover your locked thread may have a boost of its priority). For short living locks this may not be an optimal solution because to suspend/resume a thread is time and resource consuming. For this reason you can set a spin; before suspending your thread the OS will consume a little bit of time doing nothing on that thread, it may give the time to lock owner to complete its job and release the thred. To use this you have to initialize your Critical Section with InitializeCriticalSectionAndSpinCount instead of the InitializeCriticalSection.

If you plan to use a Critical Section you may consider to wrap everything needed in a class, you'll use variables scope to do everything and your code will be more clear (this is just an example, a true implementation cannot be so naive):

class critical_section
{
public:
 critical_section()
 {
  // Here you may use InitializeCriticalSectionAndSpinCount
  InitializeCriticalSection(&_cs);

  // You may not need this behavior, anyway here when you create
  // the object you acquire the lock too
  EnterCriticalSection(&_cs);
 }

 ~critical_section()
 {
   LeaveCriticalSection(&_cs);
   DeleteCriticalSection(&cs);
 }

private:
 CRITICAL_SECTION _cs;
};

Under Unix/Linux (or you want to be portable) you should use the pthread.h functions for Mutex.

Lock may not be enough

This is just the first step, if your application logs a lot you may slow down all threads waiting for log (don't do anything a priori, check and profile). If this is the case you should create a queue, your logText() function simply push a new (pre-formatted) item in the queue and calls PulseEvent to signal an event (created with CreateEvent). You'll have a second thread waiting for that event with WaitForSingleObject, your thread will wake-up and it'll pop an item from the queue. You may still need some locking mechanism (or you may write your own non-blocking concurrent queue to avoid any lock. This solution is faster and it doesn't use locks of any sort but I think you should do something like that only if you want to study the topic (threads), usually for a simple log requirement you do not need to add such complexity.