diff options
author | Kjell Ahlstedt <kjell.ahlstedt@bredband.net> | 2012-04-04 14:18:29 +0200 |
---|---|---|
committer | Kjell Ahlstedt <kjell.ahlstedt@bredband.net> | 2012-04-04 14:18:29 +0200 |
commit | 0f64aef58ac9d8ad1604800d3ba74bb998f63987 (patch) | |
tree | 7998f601d603162093624cd01533bfa79c301538 /glib/glibmm | |
parent | ac40deaa6e02797fb228e960dfff8c466f967c32 (diff) | |
download | glibmm-0f64aef58ac9d8ad1604800d3ba74bb998f63987.tar.gz |
Glib::Dispatcher: Don't send messages to a deleted Dispatcher.
* glib/glibmm/dispatcher.h: Add missing usage rules.
* glib/glibmm/dispatcher.cc: Avoid delivering messages to deleted Dispatchers.
Don't block message delivery while a second main loop is running.
Bug #651942.
Diffstat (limited to 'glib/glibmm')
-rw-r--r-- | glib/glibmm/dispatcher.cc | 87 | ||||
-rw-r--r-- | glib/glibmm/dispatcher.h | 7 |
2 files changed, 82 insertions, 12 deletions
diff --git a/glib/glibmm/dispatcher.cc b/glib/glibmm/dispatcher.cc index 6bf82430..f6257f36 100644 --- a/glib/glibmm/dispatcher.cc +++ b/glib/glibmm/dispatcher.cc @@ -27,6 +27,7 @@ #include <cerrno> #include <fcntl.h> #include <glib.h> +#include <set> #ifdef G_OS_WIN32 # include <windows.h> @@ -126,8 +127,10 @@ class DispatchNotifier : public sigc::trackable public: ~DispatchNotifier(); - static DispatchNotifier* reference_instance(const Glib::RefPtr<MainContext>& context); - static void unreference_instance(DispatchNotifier* notifier); + static DispatchNotifier* reference_instance( + const Glib::RefPtr<MainContext>& context, const Dispatcher* dispatcher); + static void unreference_instance( + DispatchNotifier* notifier, const Dispatcher* dispatcher); void send_notification(Dispatcher* dispatcher); @@ -139,6 +142,8 @@ protected: private: static Glib::Threads::Private<DispatchNotifier> thread_specific_instance_; + std::set<const Dispatcher*> deleted_dispatchers_; + long ref_count_; Glib::RefPtr<MainContext> context_; #ifdef G_OS_WIN32 @@ -152,6 +157,7 @@ private: void create_pipe(); bool pipe_io_handler(Glib::IOCondition condition); + bool pipe_is_empty(); // noncopyable DispatchNotifier(const DispatchNotifier&); @@ -165,6 +171,7 @@ Glib::Threads::Private<DispatchNotifier> DispatchNotifier::thread_specific_insta DispatchNotifier::DispatchNotifier(const Glib::RefPtr<MainContext>& context) : + deleted_dispatchers_(), ref_count_ (0), context_ (context), #ifdef G_OS_WIN32 @@ -185,8 +192,20 @@ DispatchNotifier::DispatchNotifier(const Glib::RefPtr<MainContext>& context) #else const int fd = fd_receiver_; #endif - context_->signal_io().connect(sigc::mem_fun(*this, &DispatchNotifier::pipe_io_handler), - fd, Glib::IO_IN); + // The following code is equivalent to + // context_->signal_io().connect( + // sigc::mem_fun(*this, &DispatchNotifier::pipe_io_handler), fd, Glib::IO_IN); + // except for source->set_can_recurse(true). + + const Glib::RefPtr<IOSource> source = IOSource::create(fd, Glib::IO_IN); + + // If the signal emission in pipe_io_handler() starts a new main loop, + // the event source shall not be blocked while that loop runs. (E.g. while + // a connected slot function shows a modal dialog box.) + source->set_can_recurse(true); + + source->connect(sigc::mem_fun(*this, &DispatchNotifier::pipe_io_handler)); + g_source_attach(source->gobj(), context_->gobj()); } catch(...) { @@ -248,7 +267,8 @@ void DispatchNotifier::create_pipe() } // static -DispatchNotifier* DispatchNotifier::reference_instance(const Glib::RefPtr<MainContext>& context) +DispatchNotifier* DispatchNotifier::reference_instance + (const Glib::RefPtr<MainContext>& context, const Dispatcher* dispatcher) { DispatchNotifier* instance = thread_specific_instance_.get(); @@ -261,6 +281,17 @@ DispatchNotifier* DispatchNotifier::reference_instance(const Glib::RefPtr<MainCo { // Prevent massive mess-up. g_return_val_if_fail(instance->context_ == context, 0); + + // In the possible but unlikely case that a new dispatcher gets the same + // address as a newly deleted one, if the pipe still contains messages to + // the deleted dispatcher, those messages will be delivered to the new one. + // Not ideal, but perhaps the best that can be done without breaking ABI. + // The alternative would be to remove the following erase(), and risk not + // delivering messages sent to the new dispatcher. + //TODO: When we can break ABI, a better solution without this drawback can + // be implemented. See https://bugzilla.gnome.org/show_bug.cgi?id=651942 + // especially comment 16. + instance->deleted_dispatchers_.erase(dispatcher); } ++instance->ref_count_; // initially 0 @@ -269,13 +300,22 @@ DispatchNotifier* DispatchNotifier::reference_instance(const Glib::RefPtr<MainCo } // static -void DispatchNotifier::unreference_instance(DispatchNotifier* notifier) +void DispatchNotifier::unreference_instance( + DispatchNotifier* notifier, const Dispatcher* dispatcher) { - DispatchNotifier *const instance = thread_specific_instance_.get(); + DispatchNotifier* const instance = thread_specific_instance_.get(); // Yes, the notifier argument is only used to check for sanity. g_return_if_fail(instance == notifier); + if (instance->pipe_is_empty()) + // No messages in the pipe. No need to keep track of deleted dispatchers. + instance->deleted_dispatchers_.clear(); + else + // There are messages in the pipe, possibly to the deleted dispatcher. + // Keep its address, so pipe_io_handler() can avoid delivering messages to it. + instance->deleted_dispatchers_.insert(dispatcher); + if(--instance->ref_count_ <= 0) { g_return_if_fail(instance->ref_count_ == 0); // could be < 0 if messed up @@ -331,6 +371,18 @@ void DispatchNotifier::send_notification(Dispatcher* dispatcher) #endif /* !G_OS_WIN32 */ } +bool DispatchNotifier::pipe_is_empty() +{ +#ifdef G_OS_WIN32 + return notify_queue_.empty(); +#else + PollFD poll_fd(fd_receiver_, Glib::IO_IN); + // GPollFD*, number of file descriptors to poll, timeout (ms) + g_poll(poll_fd.gobj(), 1, 0); + return (poll_fd.get_revents() & Glib::IO_IN) == 0; +#endif +} + bool DispatchNotifier::pipe_io_handler(Glib::IOCondition) { DispatchNotifyData data; @@ -385,6 +437,21 @@ bool DispatchNotifier::pipe_io_handler(Glib::IOCondition) g_return_val_if_fail(data.notifier == this, true); + // Drop the received message, if it is addressed to a deleted dispatcher. + const bool drop_message = + (deleted_dispatchers_.find(data.dispatcher) != deleted_dispatchers_.end()); + + // If the pipe is empty, there can be no messages to deleted dispatchers. + // No reason to keep track of them any more. + if (!deleted_dispatchers_.empty() && pipe_is_empty()) + deleted_dispatchers_.clear(); + + if (drop_message) + { + g_warning("Dropped dispatcher message as the dispatcher no longer exists"); + return true; + } + // Actually, we wouldn't need the try/catch block because the Glib::Source // C callback already does it for us. However, we do it anyway because the // default return value is 'false', which is not what we want. @@ -405,18 +472,18 @@ bool DispatchNotifier::pipe_io_handler(Glib::IOCondition) Dispatcher::Dispatcher() : signal_ (), - notifier_ (DispatchNotifier::reference_instance(MainContext::get_default())) + notifier_ (DispatchNotifier::reference_instance(MainContext::get_default(), this)) {} Dispatcher::Dispatcher(const Glib::RefPtr<MainContext>& context) : signal_ (), - notifier_ (DispatchNotifier::reference_instance(context)) + notifier_ (DispatchNotifier::reference_instance(context, this)) {} Dispatcher::~Dispatcher() { - DispatchNotifier::unreference_instance(notifier_); + DispatchNotifier::unreference_instance(notifier_, this); } void Dispatcher::emit() diff --git a/glib/glibmm/dispatcher.h b/glib/glibmm/dispatcher.h index e92512f3..96721651 100644 --- a/glib/glibmm/dispatcher.h +++ b/glib/glibmm/dispatcher.h @@ -48,6 +48,9 @@ class DispatchNotifier; * @li The Dispatcher object must be instantiated by the receiver thread. * @li The Dispatcher object should be instantiated before creating any of the * sender threads, if you want to avoid extra locking. + * @li The Dispatcher object must be deleted by the receiver thread. + * @li All Dispatcher objects instantiated by the same receiver thread must + * use the same main context. * * Notes about performance: * @@ -74,7 +77,7 @@ public: * @throw Glib::FileError */ Dispatcher(); - + /** Create new Dispatcher instance using an arbitrary main context. * @throw Glib::FileError */ @@ -88,7 +91,7 @@ public: private: sigc::signal<void> signal_; - DispatchNotifier* notifier_; + DispatchNotifier* notifier_; // noncopyable Dispatcher(const Dispatcher&); |