[pan2] Avoid clearing a timer event source id which is by now invalid.



commit 9ca587fd680862dd17017e92b52b8ca5edb1e924
Author: Olaf Seibert <rhialto falu nl>
Date:   Thu Mar 17 22:27:03 2016 +0100

    Avoid clearing a timer event source id which is by now invalid.
    
    (Pan:3083): GLib-CRITICAL **: Source ID 41 was not found when attempting to remove it
    
    Scenario:
      GUI::do_read_selected_group() ("periodically save our state") calls
      DataImpl::save_state() calls
      DataImpl::save_newsrc_files(DataIO& data_io)
    which calls g_source_remove(newsrc_autosave_id) but does not clear
    the variable.
    
    Later, DataImpl::mark_read(...) does not re-start the timer, because
    newsrc_autosave_id is not 0.
    
    Later, when quitting Pan, DataImpl::~DataImpl () also calls
      DataImpl::save_newsrc_files(DataIO& data_io)
    which again tries to call g_source_remove(...). But by now the value
    of newsrc_autosave_id is invalid.
    
    While looking at this I removed the unneeded in_newsrc_cb which can
    be avoided by clearing newsrc_autosave_id before calling
    save_newsrc_files(...) rather than after. I'm not sure if the event
    source really can't be removed from within its own callback, but I'm
    not trying that out.

 pan/data-impl/data-impl.cc |    1 -
 pan/data-impl/data-impl.h  |    9 +++++----
 pan/data-impl/groups.cc    |    4 +++-
 pan/data-impl/headers.cc   |    3 +--
 4 files changed, 9 insertions(+), 8 deletions(-)
---
diff --git a/pan/data-impl/data-impl.cc b/pan/data-impl/data-impl.cc
index fc2dbf3..ee940a3 100644
--- a/pan/data-impl/data-impl.cc
+++ b/pan/data-impl/data-impl.cc
@@ -78,7 +78,6 @@ DataImpl :: DataImpl (const StringView& cache_ext, Prefs& prefs, bool unit_test,
   _descriptions_loaded (false),
   newsrc_autosave_id (0),
   newsrc_autosave_timeout (0),
-  in_newsrc_cb (false),
   _rules_filter (prefs.get_flag("rules-autocache-mark-read", false),
                  prefs.get_flag("rules-auto-dl-mark-read", false),
                  prefs.get_flag("rules-autocache-mark-read", false))
diff --git a/pan/data-impl/data-impl.h b/pan/data-impl/data-impl.h
index 08cfaa7..d539bc2 100644
--- a/pan/data-impl/data-impl.h
+++ b/pan/data-impl/data-impl.h
@@ -685,16 +685,17 @@ namespace pan
             RulesFilter   _rules_filter;
 
     private:
-      guint newsrc_autosave_id;
+      mutable guint newsrc_autosave_id;
       guint newsrc_autosave_timeout;
     public:
-      bool in_newsrc_cb;
       void set_newsrc_autosave_timeout(guint seconds)
         {newsrc_autosave_timeout = seconds;}
       void save_newsrc_files()
-      {
-        save_newsrc_files(*_data_io);
+      { // Called from  rc_as_cb(...).
+        // The newsrc_autosave_id is now (soon) invalid since the timeout will be
+        // cancelled when our caller returns FALSE. So forget about it already.
         newsrc_autosave_id = 0;
+        save_newsrc_files(*_data_io);
       }
   };
 }
diff --git a/pan/data-impl/groups.cc b/pan/data-impl/groups.cc
index 9f1cfe5..38df007 100644
--- a/pan/data-impl/groups.cc
+++ b/pan/data-impl/groups.cc
@@ -202,8 +202,10 @@ DataImpl :: load_newsrc_files (const DataIO& data_io)
 void
 DataImpl :: save_newsrc_files (DataIO& data_io) const
 {
-  if (!in_newsrc_cb && newsrc_autosave_id)
+  if (newsrc_autosave_id) {
     g_source_remove( newsrc_autosave_id );
+    newsrc_autosave_id = 0;
+  }
 
   if (_unit_test)
     return;
diff --git a/pan/data-impl/headers.cc b/pan/data-impl/headers.cc
index 13af2d1..7d88111 100644
--- a/pan/data-impl/headers.cc
+++ b/pan/data-impl/headers.cc
@@ -873,9 +873,8 @@ namespace
   gboolean nrc_as_cb(gpointer ptr)
   {
     DataImpl *data = static_cast<DataImpl*>(ptr);
-    data->in_newsrc_cb = true;
     data->save_newsrc_files();
-    data->in_newsrc_cb = false;
+
     return FALSE;
   }
 }


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