C++ std::list as static member variable -- concurrency issues?

I wrote a class in charge of keeping track of error messages, which uses a static std::list to keep track of the messages. I subclass most other classes from this, to give them the ability to write to a common error log. At program close the error log is written to a file. This is a multithreaded application however ... it occurred to me I might be asking for problems here ...

class ErrorLogger
{
public:  
  void writeErrorMessage( string message );
  // ... etc
private:
  std::list<string> _theErrorMessages;
  // ... etc
};

I've been deriving several classes from this. Superficial example:

class MultiThreadingWidget : public ErrorLogger
{
public:
  void run()
  {
     // ...
     if( !isWorking ) 
     { writeErrorMessage( "MultiThreadingWidget::run failed()..."); };
     // ...
  }
};

To be frank, I'm not terribly concerned about concurrency issues as they affect the error log -- during the multithreaded part of t开发者_运维问答he run, if there's an error worthy of being written to the log, it's almost certainly a fatal error -- but I am concerned about performance issues.

Would it be better making the ErrorLog a Singleton pattern, or is that for all practical purposes the same thing?

Exceptions are not an option for this project.

Thanks in advance!

I wrote a class in charge of keeping track of error messages, which uses a static std::list to keep track of the messages.

it's not declared as static -- which is it?

EDIT: saw that was a typo, and that it should have been declared static. however, the suggestion at the end of the response may help you attain the best speed without introducing threading errors.

To be frank, I'm not terribly concerned about concurrency issues as they affect the error log -- during the multithreaded part of the run, if there's an error worthy of being written to the log, it's almost certainly a fatal error -- but I am concerned about performance issues.

your concerns are backwards -- there's no reason to introduce a bug which is difficult to track and can be easily avoided.

Q: if it's almost certainly fatal and thus (hopefully) quite rare, what will a lock cost during regular execution?

A: only some memory (for the lock). you will not write to it often.

Q: why would that cost be significant at this stage in execution?

A: the program has probably encountered something fatal

Q: why is a properly written parallel program considered to be too slow, or even slower than a single threaded implementation on typical hardware?

A: they're not. just write your program for current and future machines. ok, there are some cases where this is not good, but i suspect that is not the case since you're writing other parts of the program for concurrent execution.

you should write your implementation for concurrency, if performance really is important here.

Would it be better making the ErrorLog a Singleton pattern, or is that for all practical purposes the same thing?

no. create multiple instances which hold their own messages, if that's how you prefer to classify messages. multiple instances will reduce contest for the logger types if: 1) speed is important 2) you really have a lot of messages to log

using multiple loggers, your caller won't wait to acquire a lock as often, if you have a lot of messages to write and the messages are pushed from multiple threads.

at termination, gather all the messages from all loggers and write them to disk. this way the different loggers (with different contexts) can output their messages by category (instance), or time (if you prefer).

if you know it's fatal, then reconsider an immediate write (as 0xC0DEFACE suggests).

First never wait to log, do it ASAP. If you get a crash or something all unlogged messages are gone, so get them into that log file!

Secondly yes, trying to write to the one file from multiple threads will be problematic and having a lone instance, like a singleton to handle all the operations is a good idea. Just make sure that any writing to the log file or to the message queue are only done one thread at a time, or you WILL get difficult to track down thread related crashes/issues.

You should consider switching to a global logging function or class -- it seems odd to inherit from a class just to gain access to a logging facility. This won't make a difference for threading, but it'll make your code less prone to maintenance issues later on.

With that said: std::list is not guaranteed to be thread-safe. You can resolve this by using a mutex (e.g., using pthreads or whatever threading library you are already using), and locking it before any writes to the error log. If you're worried about performance, you can do one of two things:

1. Eliminate logging for non-debug builds. This way, you don't incur any overhead when you don't want it. Note that this is harder-than-strictly-necessary to implement if you inherit from your debugging class.

2. Eliminate locking some other way. You could let each thread have its own separate log, to be joined with the main log at periodic intervals.