-1
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?

Willi Mentzel
  • 27,862
  • 20
  • 113
  • 121
A.Martino
  • 41
  • 1
  • 7
  • 4
    This question doesn't make sense. The constructor runs *before* the object's lifetime begins. How can an object *that doesn't exist* be accessed by multiple threads at once? – Kerrek SB May 20 '16 at 10:23
  • @KerrekSB Your comment should have been an answer! As a side note: I asked a question about how object life time begins before(http://stackoverflow.com/questions/20409500/what-does-the-c-standard-mean-regarding-object-lifetime-begins) and got some interesting answers. What does "initialization complete" in the standard mean? :) – mantler May 20 '16 at 10:39
  • 1
    @GeorgeAl: What? How? When? – Kerrek SB May 20 '16 at 10:39
  • @mantler: It means that the first constructor has returned. – Kerrek SB May 20 '16 at 10:39
  • @KerrekSB Yes, but I found it interesting to see how I can figure that out by reading the standard. If I remember correctly, the crucial information was located in some other place about what "initialization complete" actually means. – mantler May 20 '16 at 10:41
  • @KerrekSB, unless im wrong `void myfoo() { static log_string my_obj; my_obj.do_stuff(); }`. What if 100 threads call this function? He doesn't specify if its `C++11` and which compiler he is using. EDIT : Sorry I just saw the `std::lock_guard` !!! My bad!!! – george_ptr May 20 '16 at 10:45
  • @GeorgeAl: static variables are initialized only once. – Kerrek SB May 20 '16 at 10:47
  • 1
    @KerrekSB, fyi http://stackoverflow.com/a/10586040/6271671 – george_ptr May 20 '16 at 10:50
  • @hyde: Thanks, cleaning up... – Kerrek SB May 21 '16 at 11:03

1 Answers1

2

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

hyde
  • 60,639
  • 21
  • 115
  • 176