[glibmm/glibmm-2-60] Fix callback races in glibmm when source is destructed



commit 42590b5a0dda3f54f87b5fa3b6fb8a59cde49ec8
Author: Dainis Jonitis <jonitis gmail com>
Date:   Thu Apr 18 15:51:24 2019 +0200

    Fix callback races in glibmm when source is destructed
    
    It is normal situation when glib main loop iterates sources on one
    thread where it checks whether source is still active and its callback
    functions can be called and glibmm Source being destroyed on other
    thread. Glibmm should check once again that callback_data and
    callback_funcs fields are still valid and GSource was not marked
    as inactive while its callback handlers are called.
    
    Fixes #41

 glib/glibmm/main.cc | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)
---
diff --git a/glib/glibmm/main.cc b/glib/glibmm/main.cc
index cffa0e0a..9eae11af 100644
--- a/glib/glibmm/main.cc
+++ b/glib/glibmm/main.cc
@@ -192,13 +192,23 @@ SourceCallbackData::destroy_notify_callback(void* data)
 static SourceCallbackData*
 glibmm_source_get_callback_data(GSource* source)
 {
-  g_return_val_if_fail(source->callback_funcs != nullptr, nullptr);
+  /* There is race between iteration of sources in main loop
+   * that checks whether they are still active and source
+   * destruction happening in other threads.
+   */
+  gpointer callback_data = source->callback_data;
+  GSourceCallbackFuncs* callback_funcs = source->callback_funcs;
+
+  g_return_val_if_fail(callback_funcs != nullptr, nullptr);
+
+  if (g_source_is_destroyed(source))
+    return nullptr;
 
   GSourceFunc func;
   void* user_data = nullptr;
 
   // Retrieve the callback function and data.
-  (*source->callback_funcs->get)(source->callback_data, source, &func, &user_data);
+  (*callback_funcs->get)(callback_data, source, &func, &user_data);
 
   return static_cast<SourceCallbackData*>(user_data);
 }
@@ -981,7 +991,10 @@ Source::~Source() noexcept
   if (gobject_)
   {
     SourceCallbackData* const data = glibmm_source_get_callback_data(gobject_);
-    data->wrapper = nullptr;
+
+    if (data) {
+      data->wrapper = nullptr;
+    }
 
     GSource* const tmp_gobject = gobject_;
     gobject_ = nullptr;
@@ -1001,7 +1014,10 @@ Source::connect_generic(const sigc::slot_base& slot)
   // Don't override the callback data.  Reuse the existing one
   // calling SourceCallbackData::set_node() to register conn_node.
   SourceCallbackData* const data = glibmm_source_get_callback_data(gobject_);
-  data->set_node(conn_node);
+
+  if (data) {
+    data->set_node(conn_node);
+  }
 
   conn_node->install(gobject_);
   return connection;
@@ -1043,7 +1059,7 @@ inline // static
   Source::get_wrapper(GSource* source)
 {
   SourceCallbackData* const data = glibmm_source_get_callback_data(source);
-  return data->wrapper;
+  return (data) ? data->wrapper : nullptr;
 }
 
 // static
@@ -1053,7 +1069,7 @@ Source::prepare_vfunc(GSource* source, int* timeout)
   try
   {
     Source* const self = get_wrapper(source);
-    return self->prepare(*timeout);
+    return (self) ? self->prepare(*timeout) : 0;
   }
   catch (...)
   {
@@ -1070,7 +1086,7 @@ Source::check_vfunc(GSource* source)
   try
   {
     Source* const self = get_wrapper(source);
-    return self->check();
+    return (self) ? self->check() : 0;
   }
   catch (...)
   {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]