C++ iostream Corruption using stringstream

Question

I'm trying to write a really simple thread-safe logger. Ideally, I wanted it to work just like std::cout, wherein you could just overload the << operator and have everything magically show up in the log. I'm on a Windows machine, so here's the approach I tried:

// Threadsafe logger
class Logger
{
public:
  Logger()
  {
    InitializeCriticalSection(&s);
  }

  ~Logger()
  {
    DeleteCriticalSection(&s);
  }

  void Log(std::ostream const& os)
  {
    EnterCriticalSection(&s);
    //std::cout << static_cast<std::stringstream const&>(os).str();
    std::cout << os.rdbuf();
    LeaveCriticalSection(&s);
  }

private:
  CRITICAL_SECTION s;
};

Notice that I've tried two approaches to the Log() function. The reason I accept an ostream is because that's what a stringstream seems to produce after the << operator is called. Both variants of the Log() function fail in the same way when I run this code:

#include <iostream>
#include <sstream>
#include <Windows.h>

int main(int argc, char* argv[])
{
  Logger logger;
  //logger.Log(std::stringstream("Test"));
  logger.Log(std::stringstream("Another ") << "test");
  std::cin.get();
}

Outputting the first line ("Test") works correctly and displays properly using both variants of the Log function. The second line outputs a mangled output:

testher

which is obviously test written over Another. What am I missing about the way these streams work? I tried making a flush call hoping that would fix things, but it did nothing.

How can I get this attempt at a thread-safe logger working correctly with the streams?

Answer 1

Using variadic templates:

void Log_impl(std::ostream &os) {} // recursion base case

template<typename T,typename... Us>
void Log_impl(std::ostream &os,T &&t,Us &&... us) {
    os << std::forward<T>(t);
    Log_impl(os,std::forward<Us>(us)...);
}

template<typename... Ts> void Log(Ts &&... ts) {
    std::stringstream ss;
    Log_impl(ss,std::forward<Ts>(ts)...);
    fprintf(stdout,"%s\n",ss.str().c_str()); // thread safe output
}

usage:

Log("Another"," test ",100);

I haven't actually tested this code...

Answer 2

At least in my opinion, this approach to the problem is at least somewhat clumsy to use, since it requires you to create an auxiliary ostream object of some sort, stream data into it, and then pass that to your log. That doesn't seem to fit very closely with what you've said you'd really prefer.

I'm also a bit less than excited about how you've done your thread-locking code. Just for example, if you get an exception during your std::cout << os.rdbuf();, you can exit the scope without leaving the critical section.

I think I'd start with a thin wrapper around a critical section, add an RAII-style class to lock the critical section (and automatically unlock it when it goes out of scope), then use those in implementing the Log class. Further, I'd cheat and have the Log class use a template member function to accept almost any type of output in one fell swoop:

Edit: After some more thought, I've decided to embrace the notion that every problem can be solved with another level of indirection. To do that, I've added an intermediate transaction that encapsulates chaining the output of a number of items into a stringstream, then writing out that result as a thread-safe transaction.

#include <windows.h>
#include <iostream>
#include <sstream>

class crit_sect {
    CRITICAL_SECTION cs;

    void lock() { EnterCriticalSection(&cs); }
    void unlock() { LeaveCriticalSection(&cs); }
    friend class lock;

    crit_sect(crit_sect const &); /* = delete; */
    crit_sect &operator=(crit_sect const &other); /* = delete; */
public:
    crit_sect() { InitializeCriticalSection(&cs); }
    ~crit_sect() { DeleteCriticalSection(&cs); }
};

class lock {
    crit_sect &cs;
public:
    lock(crit_sect &c) : cs(c) { cs.lock(); }
    ~lock() { cs.unlock(); }
};

class transaction {
    std::ostringstream buffer;
public:
    transaction(std::string const &s="") : buffer(s, std::ios::out | std::ios::ate) {}

    template <class T>
    transaction &operator<<(T const &t) {
        buffer << t;
        return *this;
    }

    friend std::ostream &operator<<(std::ostream &os, transaction const &t) {
        return os << t.buffer.str();
    }
};

class Log {
    std::ostream &out;
    crit_sect mutex;
public:
    Log(std::ostream &sink) : out(sink) { }

    template <class T>
    void operator<<(T const &t) {
        lock l(mutex);
        out << t;
    }    
};

int main() {
    Log l(std::cout);

    l << "This is a string\n";

    l << (transaction("Another ") << "Test");
    return 0;
}

Since the log type doesn't support chaining, any attempt at chaining the output without using a transaction will fail (won't compile). Compared to the original, usage is still a bit cleaner though -- the extra parameters needed for the ostringstream ctor are hidden, and the name transaction clarifies what is being done, or rather, accomplished.