A simple multithreaded FileLogger in C++ In order to learn about multithreading programming in C++, I am implementing a basic multithreaded logger.

I use a std::deque to store messages inside a FileLogger class. Each time a thread logs a message, that message is pushed to the back of the deque.

In a separate thread the FileLogger checks if there are any messages in the deque and if so writes them to file.

Access to the deque is guarded by a mutex.

In order to make it easy to log from anywhere, the logger is implemented as a singleton.

Is my code correct? How can it be improved?

// FileLogger.h:
class FileLogger
{
public:
    static void initialize(const char* filePath) { // called by main thread before any threads are spawned
        instance_ = new FileLogger(filePath);
    }
    static FileLogger* instance() { // called from many threads simultaneously
        return instance_;
    }
    void log(const std::string &amp;msg);
private:
    FileLogger(const char* filePath);
    void writeToFile();
    static FileLogger*     instance_;
    std::deque&lt;std::string&gt; messages;
    std::mutex         messagesMutex; // lock/unlock this each time messages is pushed or popped
    std::ofstream               fout;
    std::thread         writerThread;
};
// FileLogger.cpp:
FileLogger* FileLogger::instance_ = nullptr;

void FileLogger::writeToFile() {
    for (;;) {
        std::string message;
        while (messages.empty()) {
            std::this_thread::sleep_for(std::chrono::nanoseconds(10));
        }
        messagesMutex.lock();
        message = messages.front();
        messages.pop_front();
        messagesMutex.unlock();
        fout &lt;&lt; message &lt;&lt; std::endl &lt;&lt; std::flush;
    }
}

FileLogger::FileLogger(const char* filePath) {
    fout.open(filePath);
    std::thread t(&amp;FileLogger::writeToFile, this);
    writerThread = std::move(t);
}

void FileLogger::log(const std::string &amp;msg) {
    std::lock_guard&lt;std::mutex&gt; lg(messagesMutex);
    messages.push_back(msg);
}
Just up front, your code doesn't have any blatant bugs which would prevent its use. There are a few things that you could do to improve it though:


One thing that I'd try to avoid is singletons. These are effectively globals and bring with them almost all their problems. In particular, it prevents isolating code for unit tests. The alternative is that you pass a logger to the constructor of every object that needs it. This is called dependency injection, as a keyword for future research.
Instead of fout.open(path), you could pass the filename to the constructor of fout using the initializer list.
Your logger isn't copyable or assignable due to its members. I'd make that explicit though.
When writing, std::endl already flushes the stream, so the explicit std::flush is redundant.
The biggest issue is the way your writing thread operates. In effect, it wakes up every 10ns (that's 100.000.000 times per second) and looks for work (a.k.a. polling). Firstly, I can't imagine that this fast response time is actually relevant to a logger. If you have 0.1s delay between writing to the logger and seeing the data in the file, that should be enough by far. However, that's not a solution. What you actually want there is a condition variable, which allows you to only wake up the writer thread when actual works needs to be done.
There is a comment lock/unlock this each time messages is pushed or popped, which is already in the right direction. What has helped me in the past is to document which mutex protects which data. It may even be a good idea to put just those parts into a separate struct in some cases, just to make it clearer. Also, for all other members, document who is allowed to access them. For example, fout doesn't require synchronization, because it is exclusively accessed by the writer thread.
A minor fault, but still a fault is also that you access the queue without locking to check if it is non-empty. Since this typically only involves a simple pointer comparison (though that's implementation-defined), it will probably not cause issue. Using a condition variable as sketched below fixes this part as well.
You could implement this using lock-free algorithms. This is a complete rewrite though and not "smoothing out the rough edges" of your current approach. Just keep that in mind as future research project perhaps.
A minor flaw is that there is no way to shut down the logger. This is probably harmless, but it will probably trigger memory debuggers (leak detectors). If you decide not to implement shutdown, you don't need writerThread though, just call t.detach() after starting.
If your target is C++ 17 or later, you could also use the filesystem library which has a dedicated path type. I'd generally prefer that over raw char* string. Even without C++ 17, I'd use std::string, unless I have good reasons not to.
Concerning pushing on the queue, consider using emplace_back() instead, to avoid one more copying operation.
Concerning removal from the queue, there is a trick: Create a second, local queue (initially empty) and swap that with the member (holding a lock for the time of that operation, of course). Then you have all the content in a local object that you can process at leasure, without having to worry about locking. This becomes important when retrieving multiple messages at once, so that you don't repeatedly lock/unlock without need. This can lead to "thread convoys": Imagine one thread perpetually writing messages using lock/push/unlock triples, so it becomes a stream of lock/push/unlock/lock/push/unlock... Now, if this thread ever gets scheduled out while it holds the lock and the writer thread is scheduled in, it will then itself run a lock/pop/unlock/lock/pop/unlock.. stream. On the first lock, it will block until the logging thread continues. This thread will be able to write one message, before it is blocked by the writer thread. In other words, ownership of the mutex will pingpong between the two threads, wrecking performance. Also, this is hard to catch, because it both comes and goes spontaneously.


For reference, here is a sketch of the push and pop operations using a CV:

mutex mtx;
condition_variable cv;
deque&lt;string&gt; queue;

void push(string s) {
    unique_lock&lt;mutex&gt; lck(mtx);
    queue.push_back(s);
    lck.unlock();
    cv.notify_one();
}

string pop() {
     unique_lock&lt;mutex&gt; lck(mtx);
     cv.wait(lck, []{ return !queue.empty();})
     string res = queue.front();
     queue.pop_front();
     return res;
}


This is not much more than the examples given by cppreference.