Is the default constructor thread-safe in C++?

Question

class log_String {
  //These are private!
  std::vector<std::string> list;
  std::mutex m;
  log_String& operator=(const log_String &source); //Operatore assegnazione
  log_String(const log_String &source);

public:
  log_String() {}; // <---- is this thread_safe?

  void add(std::string string) {
    std::lock_guard<std::mutex> l(m);
    list.push_back(string);
  }

  void printFile(std::string file) {
    std::lock_guard<std::mutex> l(m);
    std::ofstream myfile;

    myfile.open(file);
    for (auto iterator = list.begin(); iterator != list.end(); ++iterator) {
        myfile << *iterator << "\n";
    }
  }
};

is log_String() {} thread-safe? I think that even if many threads call simultaneously log_String() {} this should not be a problem? Am i getting this wrong? If i'm wrong possible solution may be to define it private and protect the instantiation of a new object acquiring a new lock?

Answer 1

log_String() is basically a function, but it is also a constructor. So in effect its call during object creation means also calling constructors of all member variables (which have constructors), as well as constructors of all the base classes, and constructors of their member variables, recursively.

So you need to consider all the functions which get called. The two member variables, list and m, should have thread safe constructors, since they are from the standard library, and while I didn't check from the standard (draft should be freely downloadable, if you want to check yourself), things would be just crazy if they didn't have thread-safe constructors. Then there is no base class, and no code in your constructor.

Conclusion, it is thread-safe, because there's nothing in there, which would cause problems "even if many threads call simultaneously log_String()". No shared data or other shared resources visible, and if there are any shared data hidden in the member variables, they can be trusted to be done safely.

Writing thread-unsafe public constructors could be considered stupid, even evil. Still, if you had member variables or base class from 3rd party libraries, or just of your own types, and you aren't 100% sure of their quality, then it's worth it to stop and think if this kind of stupidity has been done.

---

An example code which one might plausibly write, especially for debugging purposes, and which would make things thread-unsafe:

private:
    static unsigned static_counter;

public:
    log_String() {
        ++static_counter; // not atomic operation! potential data race!
    };

For completeness: Fix for the above code would be to simply use std::atomic<unsigned> for the counter. More complex cases might require static mutexes (beware if you are using old crappy compilers (at least MSVC2010) which might have unavoidable race conditions with static data).