[git] GpgOL - branch, master, updated. gpgol-2.2.0-95-ga9c34f5

by Andre Heinecke cvs at cvs.gnupg.org
Fri Jul 20 15:39:23 CEST 2018


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GnuPG extension for MS Outlook".

The branch, master has been updated
       via  a9c34f58409c799ef22289d3bcf64b87866997c6 (commit)
       via  57dd3ee1dd73fb7f3152cee4841370090338caff (commit)
      from  aaafbc793741fc64abe02af90f22e095e3133319 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit a9c34f58409c799ef22289d3bcf64b87866997c6
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jul 20 15:24:11 2018 +0200

    Don't print error in memdbg if caller changes
    
    * src/memdbg.cpp (register_name): Don't print error
    if the name changed and the calling function was used as the name.

diff --git a/src/memdbg.cpp b/src/memdbg.cpp
index 3f68ae1..0a50513 100644
--- a/src/memdbg.cpp
+++ b/src/memdbg.cpp
@@ -49,10 +49,12 @@ register_name (void *obj, const char *nameSuggestion)
 #ifdef HAVE_W32_SYSTEM
 
   char *name = get_object_name ((LPUNKNOWN)obj);
+  bool suggestionUsed = false;
 
   if (!name && nameSuggestion)
     {
       name = strdup (nameSuggestion);
+      suggestionUsed = true;
     }
   if (!name)
     {
@@ -82,7 +84,7 @@ register_name (void *obj, const char *nameSuggestion)
                      SRCNAME, __func__, obj, it->second.c_str(),
                      sName.c_str());
           it->second = sName;
-          return true;
+          return !suggestionUsed;
         }
     }
   else

commit 57dd3ee1dd73fb7f3152cee4841370090338caff
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jul 20 15:21:34 2018 +0200

    Make UI invalidation more deterministic
    
    * src/explorer-events.cpp (hasSelection, startWatchdog, changeSeen):
    New.
    * src/mail.cpp (do_parsing): Block invalidation while parsing.
    * src/windowmessages.cpp (blockInv, unblockInv): New.
    (delayed_invalidate_ui): Handle block inv state.
    * src/windowmessages.h: Update accordingly.
    
    --
    UI Invalidation is extremly expensive and often a cause for
    crashes. So we should avoid it as much as possible. This
    code aims just to do that while still not skipping any
    required invalidation.

diff --git a/src/explorer-events.cpp b/src/explorer-events.cpp
index b9706db..d4087d1 100644
--- a/src/explorer-events.cpp
+++ b/src/explorer-events.cpp
@@ -62,6 +62,136 @@ typedef enum
     ViewSwitch = 0xF004
   } ExplorerEvent;
 
+/*
+   We need to avoid UI invalidations as much as possible as invalidations
+   can trigger reloads of mails and at a bad time can crash us.
+
+   So we only invalidate the UI after we have handled the read event of
+   a mail and again after decrypt / verify.
+
+   The problem is that we also need to update the UI when mails are
+   unselected so we don't show "Secure" if nothing is selected.
+
+   On a switch from one Mail to another we see two selection changes.
+   One for the unselect the other for the select immediately after
+   each other.
+
+   When we just have an unselect we see only one selection change.
+
+   So after we detect the unselect we switch the state in our
+   explorerMap to unselect seen and start a WatchDog thread.
+
+   That thread sleeps for 500ms and then checks if the state
+   was switched to select seen in the meantime. If
+   not it triggers the UI Invalidation in the GUI thread.
+   */
+#include <map>
+
+typedef enum
+  {
+    WatchDogActive = 0x01,
+    UnselectSeen = 0x02,
+    SelectSeen = 0x04,
+  } SelectionState;
+
+std::map<LPDISPATCH, int> s_explorerMap;
+
+gpgrt_lock_t explorer_map_lock = GPGRT_LOCK_INITIALIZER;
+
+static bool
+hasSelection (LPDISPATCH explorer)
+{
+  LPDISPATCH selection = get_oom_object (explorer, "Selection");
+
+  if (!selection)
+    {
+      TRACEPOINT;
+      return false;
+    }
+
+  int count = get_oom_int (selection, "Count");
+  gpgol_release (selection);
+
+  if (count)
+    {
+      return true;
+    }
+  return false;
+}
+
+static DWORD WINAPI
+start_watchdog (LPVOID arg)
+{
+  LPDISPATCH explorer = (LPDISPATCH) arg;
+
+  Sleep (500);
+  gpgrt_lock_lock (&explorer_map_lock);
+
+  auto it = s_explorerMap.find (explorer);
+
+  if (it == s_explorerMap.end ())
+    {
+      log_error ("%s:%s: Watchdog for unknwon explorer %p",
+                 SRCNAME, __func__, explorer);
+      gpgrt_lock_unlock (&explorer_map_lock);
+      return 0;
+    }
+
+  if ((it->second & SelectSeen))
+    {
+      log_oom_extra ("%s:%s: Cancel watchdog as we have seen a select %p",
+                     SRCNAME, __func__, explorer);
+      it->second = SelectSeen;
+    }
+  else if ((it->second & UnselectSeen))
+    {
+      log_debug ("%s:%s: Deteced unselect invalidating UI.",
+                 SRCNAME, __func__);
+      it->second = UnselectSeen;
+    }
+  gpgrt_lock_unlock (&explorer_map_lock);
+
+  do_in_ui_thread (INVALIDATE_UI, nullptr);
+  return 0;
+}
+
+static void
+changeSeen (LPDISPATCH explorer)
+{
+  gpgrt_lock_lock (&explorer_map_lock);
+
+  auto it = s_explorerMap.find (explorer);
+
+  if (it == s_explorerMap.end ())
+    {
+      it = s_explorerMap.insert (std::make_pair (explorer, 0)).first;
+    }
+
+  auto state = it->second;
+  bool has_selection = hasSelection (explorer);
+
+  if (has_selection)
+    {
+      it->second = (state & WatchDogActive) + SelectSeen;
+      log_oom_extra ("%s:%s: Seen select for %p",
+                     SRCNAME, __func__, explorer);
+    }
+  else
+    {
+      if ((it->second & WatchDogActive))
+        {
+          log_oom_extra ("%s:%s: Seen unselect for %p but watchdog exists.",
+                         SRCNAME, __func__, explorer);
+        }
+      else
+        {
+          CloseHandle (CreateThread (NULL, 0, start_watchdog, (LPVOID) explorer,
+                                     0, NULL));
+        }
+      it->second = UnselectSeen + WatchDogActive;
+    }
+  gpgrt_lock_unlock (&explorer_map_lock);
+}
 
 EVENT_SINK_INVOKE(ExplorerEvents)
 {
@@ -72,19 +202,7 @@ EVENT_SINK_INVOKE(ExplorerEvents)
         {
           log_oom_extra ("%s:%s: Selection change in explorer: %p",
                          SRCNAME, __func__, this);
-
-          HANDLE thread = CreateThread (NULL, 0, delayed_invalidate_ui, (LPVOID) this, 0,
-                                        NULL);
-
-          if (!thread)
-            {
-              log_error ("%s:%s: Failed to create invalidate_ui thread.",
-                         SRCNAME, __func__);
-            }
-          else
-            {
-              CloseHandle (thread);
-            }
+          changeSeen (m_object);
           break;
         }
       case Close:
@@ -93,6 +211,9 @@ EVENT_SINK_INVOKE(ExplorerEvents)
                          SRCNAME, __func__, this);
 
           GpgolAddin::get_instance ()->unregisterExplorerSink (this);
+          gpgrt_lock_lock (&explorer_map_lock);
+          s_explorerMap.erase (m_object);
+          gpgrt_lock_unlock (&explorer_map_lock);
           delete this;
           return S_OK;
         }
diff --git a/src/mail.cpp b/src/mail.cpp
index b7c6e02..ba134d2 100644
--- a/src/mail.cpp
+++ b/src/mail.cpp
@@ -733,6 +733,8 @@ do_parsing (LPVOID arg)
       gpgrt_lock_unlock (&dtor_lock);
       return 0;
     }
+
+  blockInv ();
   /* This takes a shared ptr of parser. So the parser is
      still valid when the mail is deleted. */
   auto parser = mail->parser ();
@@ -755,6 +757,7 @@ do_parsing (LPVOID arg)
                  SRCNAME, __func__, arg);
       gpgrt_lock_unlock (&invalidate_lock);
       gpgrt_lock_unlock (&parser_lock);
+      unblockInv();
       return 0;
     }
 
@@ -764,12 +767,14 @@ do_parsing (LPVOID arg)
                  SRCNAME, __func__, arg);
       gpgrt_lock_unlock (&invalidate_lock);
       gpgrt_lock_unlock (&parser_lock);
+      unblockInv();
       return -1;
     }
   parser->parse();
   do_in_ui_thread (PARSING_DONE, arg);
   gpgrt_lock_unlock (&invalidate_lock);
   gpgrt_lock_unlock (&parser_lock);
+  unblockInv();
   return 0;
 }
 
diff --git a/src/windowmessages.cpp b/src/windowmessages.cpp
index 3861127..9b5c46b 100644
--- a/src/windowmessages.cpp
+++ b/src/windowmessages.cpp
@@ -107,7 +107,7 @@ gpgol_window_proc (HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
             }
           case (INVALIDATE_LAST_MAIL):
             {
-              log_debug ("%s:%s: Invalidating last mail",
+              log_debug ("%s:%s: clearing last mail",
                          SRCNAME, __func__);
               Mail::clearLastMail ();
               break;
@@ -425,6 +425,8 @@ gpgrt_lock_t invalidate_lock = GPGRT_LOCK_INITIALIZER;
 
 static bool invalidation_in_progress;
 
+static int invalidation_blocked = 0;
+
 DWORD WINAPI
 delayed_invalidate_ui (LPVOID)
 {
@@ -434,13 +436,26 @@ delayed_invalidate_ui (LPVOID)
                  SRCNAME, __func__);
       return 0;
     }
-  gpgrt_lock_lock(&invalidate_lock);
+  TRACEPOINT;
   invalidation_in_progress = true;
-  /* We sleep here a bit to prevent invalidation immediately
-     after the selection change before we have started processing
-     the mail. */
-  Sleep (250);
+  gpgrt_lock_lock(&invalidate_lock);
+
+  int i = 0;
+  while (invalidation_blocked)
+    {
+      Sleep (100);
+      i++;
+
+      if (i % 10 == 0)
+        {
+          log_debug ("%s:%s: Waiting for invalidation.",
+                     SRCNAME, __func__);
+        }
+
+      /* Do we need an abort statement here? */
+    }
   do_in_ui_thread (INVALIDATE_UI, nullptr);
+  TRACEPOINT;
   invalidation_in_progress = false;
   gpgrt_lock_unlock(&invalidate_lock);
   return 0;
@@ -452,3 +467,26 @@ close_mail (LPVOID mail)
   do_in_ui_thread (CLOSE, mail);
   return 0;
 }
+
+void
+blockInv()
+{
+  invalidation_blocked++;
+  log_oom_extra ("%s:%s: Invalidation block count %i",
+                 SRCNAME, __func__, invalidation_blocked);
+}
+
+void
+unblockInv()
+{
+  invalidation_blocked--;
+  log_oom_extra ("%s:%s: Invalidation block count %i",
+                 SRCNAME, __func__, invalidation_blocked);
+
+  if (invalidation_blocked < 0)
+    {
+      log_error ("%s:%s: Invalidation block mismatch",
+                 SRCNAME, __func__);
+      invalidation_blocked = 0;
+    }
+}
diff --git a/src/windowmessages.h b/src/windowmessages.h
index 2d6da49..29364a0 100644
--- a/src/windowmessages.h
+++ b/src/windowmessages.h
@@ -92,6 +92,15 @@ do_in_ui_thread_async (gpgol_wmsg_type type, void *data, int delay = 0);
 HHOOK
 create_message_hook();
 
+/** Block ui invalidation. The idea here is to further reduce
+    UI invalidations because depending on timing they might crash.
+    So we try to block invalidation for as long as it is a bad time
+    for us. */
+void blockInv ();
+
+/** Unblock ui invalidation */
+void unblockInv ();
+
 DWORD WINAPI
 delayed_invalidate_ui (LPVOID);
 

-----------------------------------------------------------------------

Summary of changes:
 src/explorer-events.cpp | 147 +++++++++++++++++++++++++++++++++++++++++++-----
 src/mail.cpp            |   5 ++
 src/memdbg.cpp          |   4 +-
 src/windowmessages.cpp  |  50 ++++++++++++++--
 src/windowmessages.h    |   9 +++
 5 files changed, 195 insertions(+), 20 deletions(-)


hooks/post-receive
-- 
GnuPG extension for MS Outlook
http://git.gnupg.org




More information about the Gnupg-commits mailing list