Singleton in multithreaded program

[Transrian]

Hi guys ! Sorry if I don't know how to short up the subject, but I'm not used to write or speak english.

I actually got a class, called Analysis, that analyse some data, by launching some threads(about 10) to perform it.

I writed too a class Log, that write lines of text in a file. It is multithread-safe (several threads can access it at the same time), and I use it with a Singleton to don't send each time I use a function the reference of this instance of Log.
a

At this point, all work as well, but I need now to launch several instances of Analysis, but with the Singleton of my class Log, it don't work as it should be, because all logs are write in the same file, whereas each instance of Analysis should have it proper Log instance.
I have not idea of how to change this class to permet to all Analysis instances to write in differents files, without passing any reference (I just got one ID to all Analysis instances).

(I use the C++11 standart <thread> for multithreading)

If you could lead me on the right way, it should be great :)

[shadowmouse]

Do you have a reason for using a singleton rather than just a normal class that is instantiated once? From reading the constant preaching of experienced programmers on this and the sfml forum, singletons are a bad idea. Even if they don't seem so in the short term, the lack of scope control will cause many problems. I was recently reading that using singletons with threads is likely to cause many hateful problems, switch forums, and ta da!

[Transrian]

Well, I'm not sure of this point.
The Log class is only use to store String (send from threads), and then write them to file at the end of the analyse.


This is the Log.h file

typedef std::vector<String> TextQueue;

class Log{

	private:

		String name;

                // name is used as output path
		std::ofstream output;

		TextQueue queue;

	public:

		Log(const String& name);
		~Log();

		// Add line of text in container
		void push_back(const String& str);
		
		// Write text stored in file
		void write();

};

What seem's like the best ?
I was thinking about Singleton because I want to access to the Log instance easily, without passing reference of them into threads, but it isn't probably the best solution.

Do you have any idea of how to do that ?

[shadowmouse]

Without seeing your entire project, and I doubt posting something that big would help anyway, I'd suggest just making it a normal class, declared outside of main and first instantiated in main. Then just pass it by reference to all the functions it's needed in. It is only one more parameter.

[Gamer2015]

and I use it with a Singleton to don't send each time I use a function the reference of this instance of Log.

This class does not look like a singleton class but there is a way that you can still make it threadsave over multiple instances ;)

The Log class is only use to store String (send from threads), and then write them to file at the end of the analyse.

What do you mean?
Does your push_back function add the string to the TextQueue like this and after that you call write to write the entries in the queue to the file?
void push_back(const String& str) { queue.push_back(str); }

If that's true too you can
- make it singleton-thread-save (push_back has to be threadsave)
Note: if you have multiple threads calling the write function you also have to make that threadsave. This scenario doesn't make any sense because once you call write on the singleton the queue is empty.

void push_back(const String& str) { 
    static std::mutex mu; 
    std::lock_guard<std::mutex> lock(mu);
    queue.push_back(str); 
}

- make it non-singleton-thread-save (write has to be threadsave)

void write() { 
    static std::mutex mu; 
    std::lock_guard<std::mutex> lock(mu);
    for(const auto& data : queue)
        output << data;
    queue.clear();
}

to make sure everything works you can just make both methods threadsave.
Note that push_back is not allowed to be called while writing so if you don't know when you write or push_back you have to use 1 mutex for both of them, so you need 1 static mutex in the class and lock that one instead of the mutexes above.