Sign in to follow this  
Kasya

Thread-safe Logging System Help

Recommended Posts

Hello,I need help making a thread safe logging system. Here is my code (works on single thread):

[code]class Logger {
public:
typedef std::vector<LogListener *> ListenerList;

private:
boost::mutex mutex;
ListenerList listeners;

public:
Logger();
~Logger();

Logger * write(const String &line);

};

Logger * Logger::write(const String &text) {


boost::unique_lock<boost::mutex> lock(mutex);
//mutex.lock();
for(ListenerList::iterator i = listeners.begin(); i != listeners.end(); ++i) {
(*i)->write(text);
}
//mutex.unlock();
//both didn't work

return this;

}

class FileLogListener : public LogListener {
std::ofstream stream;
public:
FileLogListener(const String &str) : stream(str.c_str(), std::ios::out | std::ios::app) { }
void write(const String &text) {
stream << text << std::endl;
}

};


class CoutLogListener : public LogListener {
public:
CoutLogListener() { }
void write(const String &text) {
std::cout << text << std::endl;
}

};

class Base {
private:
//...
static Logger * logger;


public:
//...
static Logger * log() { return logger; }


};

Logger::Logger() : mutex(), listeners() {
listeners.push_back(new CoutLogListener);
listeners.push_back(new FileLogListener("file.txt"));
}


[/code]

[code]//Thread 1

void func1() {
Base::log()->write("Thread 1 Test");
}

//Thread 2

void func2() {
Base::log()->write("Thread 2 test");
}

int main() {
Base::setLogger(new Logger);
boost::thread t1(&func1);
boost::thread t2(&func2);
t1.join();
t2.join();
return 0;
}[/code]


[font="Times New Roman"][size="2"]"file.txt" becomes a mess when I run the code (thread 1 is sometimes shown twice, sometimes thread 2 is shown twice, sometimes its correct, sometimes only one is shown once). What am I doing wrong?[/size][/font]

[font="Times New Roman"][size="2"]I am on Ubuntu Linux
[/size][/font]
[font="Times New Roman"][size="2"]Thanks in advance,[/size][/font]
[font="Times New Roman"][size="2"]Gasim[/size][/font]

Share this post


Link to post
Share on other sites
I'm not a boost expert at all, but you create a local mutex in each function and afterward lock this mutex ? This would result in any locking at all. You need to share one mutex, declared outside the functions (and don't forget to unlock it afterward, if this is necessary with boost).

Share this post


Link to post
Share on other sites
Thank you for fast replies. I have updated the code. But it still doesn't work. I didn't get one right output out of ~15. [img]http://public.gamedev.net/public/style_emoticons/default/blink.gif[/img]

Can it be a hardware issue?

I am running it on Intel Atom N270 on Ubuntu Linux (don't comment please :) i know its terrible)

Share this post


Link to post
Share on other sites
I have finally figured it out. There has to be. thread.join in order to make it execute. Edited my code just in case someone needs it.

I have one question though. Is that a good idea to use locks in that scenario, considering that I there is going to be a lot of log messages during development stage? Am I going to lose performance in that case due to sync?

Share this post


Link to post
Share on other sites
Yes, you will lose performance - but there's really no good way to do this without locking. (OK, so there's some lock-free queue mechanisms you could use, but they have a lot of complications and require some major sacrifices to work.) The best solution is to log minimally, e.g. provide a "level of detail" flag that can be used to filter out log lines before you lock and try to write them to disk.

Share this post


Link to post
Share on other sites
[quote name='Gasim' timestamp='1311080302' post='4837355']
I have one question though. Is that a good idea to use locks in that scenario, considering that I there is going to be a lot of log messages during development stage? Am I going to lose performance in that case due to sync?
[/quote]
One way to save performance is to use one log file per thread. Either life with distributed logs or write some tool which merges the logs (timestamp needed in this case).

An other more complicated way is to use one log-buffer per thread and swap it once it is full or after a certain period. A collector thread will gather all full buffers and write them in the correct order out. In this case you would have only few syncronisation points. Thought you must take care in case of a crash. In this case get all buffers and write them out before exiting the application.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this