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?

Have you tried logger.Log((std::stringstream("Another ") << "test")); ? See the extra parenthesis? — std''OrgnlDave
– std''OrgnlDave, Commented Apr 11, 2012 at 16:35
I just tried that and it produced the same result. Thanks for the attempt though! I think it should be the same because all of the function parameters should be fully evaluated before calling the Log() function. — aardvarkk
– aardvarkk, Commented Apr 11, 2012 at 16:38
I upvoted edransch's answer because it describes why you're getting this result, but Jerry Coffin's answer points out an unrelated serious problem. I'd probably use standard C++ constructs (e.g., <atomic>,<mutex>, and <thread>) to solve it though since they already have the appropriate RAII sematics you want for C++. — bames53
– bames53, Commented Apr 11, 2012 at 17:14
Or just use fprintf, since that is already thread safe: fprintf(stdout,"%s\n",(std::stringstream() << "another" << " test").str().c_str()); — bames53
– bames53, Commented Apr 11, 2012 at 17:23
Agreed -- Jerry's answer goes above and beyond to better my entire approach, but edransch answers the core of the issue directly. I didn't realize fprintf was thread safe -- that's a pretty sweet one-liner solution! — aardvarkk
– aardvarkk, Commented Apr 11, 2012 at 17:27

bames53 · Accepted Answer · 2012-04-11 17:35:10Z

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...

LarryF · Accepted Answer · 2017-11-08 04:29:08Z

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.

Just to clarify, in this implementation, you'd have to make sure not to call l with chained outputs because then you lose the thread safety, right? I assume that's what you're trying to demonstrate with the buffer there...
@aardvarkk: Yes, unfortunately that's true. Just to clarify, however, you're not really losing anything -- you're just not adding something (though, admittedly, something that would be really nice to have).
It just worries me a bit that you still have to be cognisant of that fact when using the logger or it's really not a thread-safe logger anymore (at least in the sense that I imagined it). But I suppose the simple implementation I imagine is impossible because in a multithreaded world, you'd never know when logging started and stopped. For the record, I think your answer is awesome and very helpful! I definitely learned something from it (I understand RAII just a bit better now!). Unfortunately, I feel that edransch has more directly resolved the issue I was having.
@aardvarkk: Thinking about it a bit more, it's probably better to just disable chaining for this class, so the user doesn't use it by accident.
That makes a lot of sense. Kudos for creating such a solid logging solution!

edransch · Accepted Answer · 2012-04-11 16:46:39Z

The problem isn't with the logger, rather it's with your use of stringstream. When std::stringstream is initialized, the stream's position indicator is positioned at the beginning of the stream.

Now when you start writing to the string using '<<', you start writing at the position indicator, replacing whatever was there before.

To work around this, you can initialize the stringstream with std::stringstream("Another ", stringstream::in | stringstream::out | std::stringstream::ate)

(as per http://www.cplusplus.com/reference/iostream/stringstream/stringstream/ )

paquetp · Accepted Answer · 2012-04-11 16:35:58Z

<< doesn't send the null byte at the end of "test" into the temporary stringstream("Another ") you created, which is why you see "testher".

Collectives™ on Stack Overflow

C++ iostream Corruption using stringstream

4 Answers 4

Comments

6 Comments

Comments

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

6 Comments

Comments

Comments

Linked

Related