Boost thread request queue design question
Hi folks.
I want a FIFO URL request queue for an application I'm developing (or rather re-developing). I want the URL requests to be executed on a separate thread and I want them to do a callback to a member function of some class I have. So, I'm using libcurl (with a class wrapper I developed a long time ago), sigc (for callbacks) and boost (for threads). I have something that works , but I'm not sure if it's a good/safe design 'cause I haven't done threadding stuff in a long time. Nor have I really ever done a whole bunch of it in the first place.
So a brief description of the code...
main: sets up the sigc callbacks and the threadded url class. It puts two requests to be downloaded, and then sleeps for 10 seconds (the application I'm developing is a GUI, this is simply a console demonstrater).
printStuff: The callback. It simply prints the source of the webpage that was downloaded. This is just a simple function. While the application I will be using will be using a class-member callback, this demonstrates the poit pretty well.
libURLstream_thread: A templated class which should handle the threads, the posting, and the actual calling backs.
* I would appreciate feedback particularily about the doPostAndCallback member.
* Also, I don't do anything with thread member variable after using it. How dangerous is that? I assumed that once the class closes the thread variable will destruct...
* And instead of the while(locked) if thread is in use is it OK to simply do a thread.join()? I don't intend to, but it was just something that I thought about considering.
(Note, my niall::urlStream object will be static. This is because I only want one instance of it as the template arguments will be different if the callback parameters are different (which they will be) in the proper code. )
So the question: how thread unsafe is this? And what should I do (read in all likelihood) in order to make it more thread safe?
All feedback appreciated.
Code:
// System headers
#include <iostream>
#include <fstream>
#include <string>
#include <queue>
// Boost headers
#include <boost/thread.hpp>
#include <boost/date_time.hpp>
// Sigc headers
#include <sigc++/sigc++.h>
// My headers
#include <niall/urlStream.h>
namespace niall {
// A class which will post to a URL and callback to a user-defined function
// when the posting has completed
template<typename callbackType>
class libURLstream_thread {
private:
// The request type
struct requestElement {
std::string url;
std::string params;
// Overload the = operator for convenience later
requestElement &operator = ( requestElement &e ) {
// Copy the individual paramaters
url = e.url;
params = e.params;
return *this;
}
};
typedef std::queue<requestElement> requestQueue;
// For net access (static, because I only want one instance of it)
niall::urlStream stream;
// The current queue of requests
requestQueue theQueue;
// These are for thread things
bool beingUsed;
boost::mutex mutex;
boost::thread thread;
boost::condition_variable inUse;
// Push a request element to the back of the queue
void push( const requestElement &el ) {
boost::mutex::scoped_lock lock( mutex );
theQueue.push( el );
lock.unlock();
inUse.notify_one();
}
// Pop the first element from the queue, return accordingly
bool pop( requestElement &el ) {
boost::mutex::scoped_lock lock( mutex );
// Do nothing if the queue is empty, so return false
if( theQueue.empty() ) {
return false;
}
// Otherwise pop the first element
el = theQueue.front();
theQueue.pop();
return true;
}
// The real thread functionality is here
void doPostAndCallback( const requestElement &req, callbackType &callback ) {
// Wait until we're good to go
boost::mutex::scoped_lock lock( mutex );
while ( beingUsed )
inUse.wait( lock ); // possible alternative: thread.join() ?
beingUsed = true;
// Do the real posting
std::cout<< "Posting to: " << req.url << "\n";
stream.post( req.url, req.params );
std::cout<< "Done!\n\n";
// Call the callback and wait for it to finish
callback( stream.getSource() );
beingUsed = false;
}
public:
// Constructor, do nothing here
libURLstream_thread()
: beingUsed( false ) {
}
~libURLstream_thread() {
}
// Our post function. This will handle all the stuff for us
void post( const std::string &url, const std::string ¶ms, callbackType &callback ) {
// Must wait
requestElement req;
req.url = url;
req.params = params;
push( req );
// Pop the frontmost element and start the thread
if ( pop(req) ) {
thread = boost::thread( boost::bind( &libURLstream_thread::doPostAndCallback, this, req, callback ) );
}
}
};
}
void printStuff( const std::string &src ) {
std::cout<< "Source: " << src << "\n\n";
}
int main( void ) {
// Set up the callback function
typedef sigc::signal<void,std::string> urlStreamCallback;
urlStreamCallback callback;
callback.connect( sigc::ptr_fun( printStuff ) );
// And now set up the threadded posting
niall::libURLstream_thread<urlStreamCallback> stream;
stream.post( "http://www.something.com", "", callback );
stream.post( "http://www.google.com", "", callback );
Sleep( 10000 );
// just to pause it. The application is a gtkmm GUI
return 0;
}
