GNOME Bugzilla – Bug 511972
glibmm doesn't have an AsyncQueue class
Last modified: 2016-03-20 17:29:03 UTC
In glib there is the GAsyncQueue that can be used to send inter-thread objects in a thread-safe way. The Glib::Dispatcher is usefull for sending events, but not to transmit computation results, or synchronise a thread doing some logic work... but glib does. I've created my own, somewhat based on the glib one code, but using the std::queue and the glibmm thread logics... it adds an usefull interrupt() method that allow the pop() blocking methods to throw an exception if another thread called interrupt... it allows the thread to stop wainting for a push() if the user wants the thread to exit. the exception mechanism is not the one glibmm but the one of the stdlib, so this should probably be replaced... is this any usefull? do you have suggestions to make it go the 2.15 glibmm svn head
Created attachment 103707 [details] async queue implementation
Created attachment 103709 [details] my async queue implementation sorry the previous file was not the right one... here the one which work
Could you start a discussion about this on gtkmm-list, please. I'd like more opinions.
And of course it would be helpful if the documentation was in English (rather than French) and if it was formatted like the rest of glibmm.
"Could you start a discussion about this on gtkmm-list, please. I'd like more opinions." In the absence of that, basing a thread safe queue on GAsyncQueue itself in glibmm would not in my view be sensible. We have std::queue and applying thread safe wrappers to one is relatively straightforward, and the implementation that has been offered for this is quite reasonable, except that it loses strong exception safety in the pop() method as it copies to the return value after popping the value at the front of the queue (there is some discussion about this in one of the Herb Sutter's books). The normal solution to this in locked queues where everything has to be done under a single atomic lock is for the pop() method to take a reference argument and pass the value at the front of the queue by reference rather than as a return value, which avoids a copy.
So we would have to make the usual decision about whether glibmm is the right place to put this, considering that glibmm is meant for wrappers of glib. We have broken this rule already for Glib::Dispatcher and ustring::compose() though. Does this feel as useful?
chris, i understand your point about stong exception safety, and the ones about this class not being a wrapper but a try to map the interface of GAsyncAqueue into a new c++ class. I'let you decide whether this is usefull or not... i can make this implementation exception safe, as far as i know, splitting pop into a front() and a pop() method without return value (as in the std::queue) would do the job. I can also translate and improve the documentation and make it formatted like the one of glibmm. Let me know what you decide...
No you can't do it as in std::queue because the call to front() and pop() have to be done under a single lock atomically. As I say, the best thing is to pass the object to be extracted to a reference argument of the pop() method, not as a return value. That retains strong exception safety (the extraction either succeeds, or the queue remains in its former condition) and thread safety. I do not take any decisions on what goes into glibmm, that is for others. Glib::Dispatcher provides for event passing, so its main uses would be in constructing event queues that a worker thread can wait on (which is one of the main intended uses of GAsyncQueue), or for passing arguments to a method invoked by Glib::Dispatcher. For those purposes I think it would be quite useful. Blocking queues of this kind can also be used to make thread pools, but glibmm already has those. Few people writing threaded programs can do without a thread safe queue, and most people will have something like yours in their own library, so the question is whether it should go into glibmm. If it were included there are a few suggestions of taste I would make - for example I think it sensible to pass the container type as a second template parameter with a default value, as in the case of std::queue (on what should be the default value, there might be an argument in this use for std::list rather than std::deque which std::queue uses by default). In addition, I am not certain I would cause try_pop() to give rise to an exception if the queue is empty (it is not really an unexpected condition if try_pop() is being called). One approach is to have try_pop() return with a success/fail argument, and to have two pop() methods, one which blocks on a condition variable if the queue is empty (the present behaviour of your pop() method), and one which throws an exception if it is empty as per std::queue. I can see however where GAsyncQueue took you, but perhaps we do not have to follow it. You also don't need to count waiting threads to know whether to call Glib::Cond::signal() - if nothing is waiting on the condition variable the call to signal() will do nothing; however, perhaps you were doing it for some other purpose. Anyway, well done.
Created attachment 103937 [details] new version this version tries to map Glibmm formatting, adds the correct #define , the GPL header , english documentation... it adds the possibility to specify your own container like std::queue does (by default it uses std::deque like the standard says to do). it removes the waiting_threads variable, as it's not necessary to use it, may be a debug swith could it for testing purpose... it let every pop methods untouched, to keep the interface easy to use, but a reference passing result version can be added if you want to. it still don't uses the glibmm exception infrastructure...let me know if it's wanted. if you want to discuss it in the mailing list, i don't have access to it, nor the time to do it... but i'll try to keep an eye on this bug report .
Created attachment 103938 [details] sorry, formatting was done with tabs...
Created attachment 103939 [details] template definition corrections template forward definitions were not up to date.
Thanks. That's a big help (though the indenting and doxygen comment style is still not the same as the rest of glibmm). I would still like to see discussion of this on gtkmm-list, because this is not something that I currently need to use.
Currently I use Anthony William's concurrent queue described here: http://www.justsoftwaresolutions.co.uk/threading/implementing-a-thread-safe-queue-using-condition-variables.html It would be nice to see an AsyncQueue class in glibmm, as his implementation depends on boost.
Could you provide a git commit patch please, ideally with some kind of test.
Glib::Thread, Glib::Mutex, Glib::Cond and the whole Glib::Threads namespace have been deprecated in favour of the standard C++ concurrency API in C++11 and C++14. There is no thread-safe queue class in the C++ standard library. But still, hasn't a thread-safe queue class in glibmm become less relevant than it was when this bug was filed? We now try to have as little multithread support as possible in glibmm.
Agreed. We should not encourage mixing GThread and std::thread and this is not something that really needs to be part of glibmm. In fact, being in glibmm would make it less likely to be used and improved widely enough.