[git] GpgOL - branch, master, updated. gpgol-2.2.0-79-g35e6637

by Andre Heinecke cvs at cvs.gnupg.org
Fri Jul 20 10:50:51 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  35e663754f7d407adf4ae6864f41da288f11ecb9 (commit)
       via  3a0fdfd1cb67db0282f74b327ee303685b7b6f1d (commit)
       via  ff589bd30f79b606566f68446b0f2b5f3641e17b (commit)
       via  3f51209f207fc0df13ff42db57429b8c6542db9b (commit)
       via  a74cfd9861cecc8bdb2226b2dd280bf862697d2d (commit)
       via  909a57ff02cd399878d0219bd10ab25177e4f2bc (commit)
       via  5959ec68f8ae30e3085d5a180db39d7ca4d48817 (commit)
       via  af4626b1fdcebbf9fe36d4aae9c664df7780099d (commit)
       via  2e63ea41583fc7cfb3f7e1442f6d9c6a846d6d16 (commit)
       via  74c6f5d78ee6bf31fc70ef5ad0b6cd3b3656a0aa (commit)
       via  fe5ce6b8faf4503f8996f4bd10afdaa8b9aaba0c (commit)
       via  05c70803c61420cc61f566b577117522ead3f1a4 (commit)
      from  5e2078d4f6686823d9a6c7a69490ccf02144ce4c (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 35e663754f7d407adf4ae6864f41da288f11ecb9
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jul 20 10:48:02 2018 +0200

    Fix big reference leak of cipherstream / message
    
    * src/mail.cpp (get_attachment_stream_o): Release
    message and mapi_attachment after obtaining the stream.
    
    --
    This is a big leak. It probably means that Outlook kept
    the original Message fully in the memory.
    
    For this the memdbg interface might already be worth it.

diff --git a/src/mail.cpp b/src/mail.cpp
index 9192b4a..38ee1d0 100644
--- a/src/mail.cpp
+++ b/src/mail.cpp
@@ -451,8 +451,9 @@ get_attachment_stream_o (LPDISPATCH mailitem, int pos)
                      SRCNAME, __func__);
           return NULL;
         }
-      hr = message->OpenProperty (PR_BODY_A, &IID_IStream, 0, 0,
-                                  (LPUNKNOWN*)&stream);
+      hr = gpgol_openProperty (message, PR_BODY_A, &IID_IStream, 0, 0,
+                               (LPUNKNOWN*)&stream);
+      gpgol_release (message);
       if (hr)
         {
           log_debug ("%s:%s: OpenProperty failed: hr=%#lx",
@@ -473,13 +474,15 @@ get_attachment_stream_o (LPDISPATCH mailitem, int pos)
                  SRCNAME, __func__, attachment);
       return NULL;
     }
-  if (FAILED (mapi_attachment->OpenProperty (PR_ATTACH_DATA_BIN, &IID_IStream,
-                                             0, MAPI_MODIFY, (LPUNKNOWN*) &stream)))
+  if (FAILED (gpgol_openProperty (mapi_attachment, PR_ATTACH_DATA_BIN,
+                                  &IID_IStream, 0, MAPI_MODIFY,
+                                  (LPUNKNOWN*) &stream)))
     {
       log_debug ("%s:%s: Failed to open stream for mapi_attachment: %p",
                  SRCNAME, __func__, mapi_attachment);
       gpgol_release (mapi_attachment);
     }
+  gpgol_release (mapi_attachment);
   return stream;
 }
 

commit 3a0fdfd1cb67db0282f74b327ee303685b7b6f1d
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jul 20 10:46:13 2018 +0200

    Improve memdbg name output
    
    * src/memdbg.cpp (register_name): Handle name changes.
    (memdbg_addRef): Handle name change.

diff --git a/src/memdbg.cpp b/src/memdbg.cpp
index cd506dc..088767f 100644
--- a/src/memdbg.cpp
+++ b/src/memdbg.cpp
@@ -42,28 +42,53 @@ GPGRT_LOCK_DEFINE (memdbg_log);
 # include "oomhelp.h"
 #endif
 
-static void
+/* Returns true on a name change */
+static bool
 register_name (void *obj)
 {
 #ifdef HAVE_W32_SYSTEM
-  auto it = olNames.find (obj);
-
-  if (it != olNames.end())
-    {
-      return;
-    }
 
   char *name = get_object_name ((LPUNKNOWN)obj);
+
   if (!name)
     {
-      return;
+      auto it = olNames.find (obj);
+      if (it != olNames.end())
+        {
+          if (it->second != "unknown")
+            {
+              log_debug ("%s:%s Ptr %p name change from %s to unknown",
+                         SRCNAME, __func__, obj, it->second.c_str());
+              it->second = "unknown";
+              return true;
+            }
+        }
+      return false;
     }
+
   std::string sName = name;
-  olNames.insert (std::make_pair (obj, sName));
   xfree (name);
+
+  auto it = olNames.find (obj);
+  if (it != olNames.end())
+    {
+      if (it->second != sName)
+        {
+          log_debug ("%s:%s Ptr %p name change from %s to %s",
+                     SRCNAME, __func__, obj, it->second.c_str(),
+                     sName.c_str());
+          it->second = sName;
+          return true;
+        }
+    }
+  else
+    {
+      olNames.insert (std::make_pair (obj, sName));
+    }
 #else
   (void) obj;
 #endif
+  return false;
 }
 
 void
@@ -83,7 +108,11 @@ memdbg_addRef (void *obj)
   if (it == olObjs.end())
     {
       it = olObjs.insert (std::make_pair (obj, 0)).first;
-      register_name (obj);
+    }
+  if (register_name (obj) && it->second)
+    {
+      log_error ("%s:%s Name change without null ref on %p!",
+                 SRCNAME, __func__, obj);
     }
   it->second++;
 

commit ff589bd30f79b606566f68446b0f2b5f3641e17b
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jul 20 10:45:00 2018 +0200

    Change an error to debug
    
    * src/mail.cpp (Mail::installFolderEventHandler_o): Don't
    print the success message as error.

diff --git a/src/mail.cpp b/src/mail.cpp
index f2f99f4..9192b4a 100644
--- a/src/mail.cpp
+++ b/src/mail.cpp
@@ -3415,7 +3415,7 @@ Mail::installFolderEventHandler_o()
 
   if (s_folder_events_map.find (strPath) == s_folder_events_map.end())
     {
-      log_error ("%s:%s: Install folder events watcher for %s.",
+      log_debug ("%s:%s: Install folder events watcher for %s.",
                  SRCNAME, __func__, strPath.c_str());
       const auto sink = install_FolderEvents_sink (folder);
       s_folder_events_map.insert (std::make_pair (strPath, sink));

commit 3f51209f207fc0df13ff42db57429b8c6542db9b
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jul 20 10:43:02 2018 +0200

    Unref application unload / shutdown
    
    * src/gpgoladdin.cpp (GpgolAddin::shutdown): Release
    application.
    
    --
    Minor and should not have been a problem.

diff --git a/src/gpgoladdin.cpp b/src/gpgoladdin.cpp
index fa18259..cf75cc1 100644
--- a/src/gpgoladdin.cpp
+++ b/src/gpgoladdin.cpp
@@ -1164,6 +1164,9 @@ GpgolAddin::shutdown ()
                   MB_ICONINFORMATION|MB_OK);
     }
   m_shutdown = true;
+
+  gpgol_release (m_application);
+  m_application = nullptr;
 }
 
 void

commit a74cfd9861cecc8bdb2226b2dd280bf862697d2d
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jul 20 10:40:40 2018 +0200

    Fix reference error on Explorer / Explorers object
    
    * src/gpgoladdin.cpp (install_explorer_sinks): Release
    explorer and explorers after installing the sinks.
    
    --
    The sinks handle the ref counting by themself. The
    reference leak should be not a real problem because
    the sinks keep them anyway until the end of the
    program.

diff --git a/src/gpgoladdin.cpp b/src/gpgoladdin.cpp
index 31eb440..fa18259 100644
--- a/src/gpgoladdin.cpp
+++ b/src/gpgoladdin.cpp
@@ -461,9 +461,13 @@ install_explorer_sinks (LPDISPATCH application)
                          SRCNAME, __func__, sink, i);
           GpgolAddin::get_instance ()->registerExplorerSink (sink);
         }
+      gpgol_release (explorer);
     }
   /* Now install the event sink to handle new explorers */
-  return install_ExplorersEvents_sink (explorers);
+  LPDISPATCH ret = install_ExplorersEvents_sink (explorers);
+  gpgol_release (explorers);
+
+  return ret;
 }
 
 static DWORD WINAPI

commit 909a57ff02cd399878d0219bd10ab25177e4f2bc
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jul 20 10:34:00 2018 +0200

    Add memdbg handling for MAPI
    
    * src/gpgoladdin.cpp (GpgolAddin::OnConnection): Ref application.
    * src/mapihelp.cpp: Replace OpenProperty by gpgol_openProperty.
    memdbg_addRef for some MAPI functions that return an object.
    * src/mimemaker.cpp: Replace OpenProperty by gpgol_openProperty.
    * src/mlang-charset.cpp (ansi_charset_to_utf8): Mark multilang
    as reffed.
    * src/olflange.cpp (install_forms): Mark formcontainer as reffed.
    * src/oomhelp.cpp, src/oomhelp.h (gpgol_queryInterface): New.
    (get_oom_iunknown): Add ref.
    (get_oom_base_message_from_mapi): Add ref.
    * src/ribbon-callbacks.cpp (getIcon): Add ref on stream and
    change it to be automatically freed as we don't free it.

diff --git a/src/gpgoladdin.cpp b/src/gpgoladdin.cpp
index 0a0fe10..31eb440 100644
--- a/src/gpgoladdin.cpp
+++ b/src/gpgoladdin.cpp
@@ -319,6 +319,7 @@ GpgolAddin::OnConnection (LPDISPATCH Application, ext_ConnectMode ConnectMode,
   can_unload = false;
   m_application = Application;
   m_application->AddRef();
+  memdbg_addRef (m_application);
   m_addin = AddInInst;
 
   version = get_oom_string (Application, "Version");
diff --git a/src/mapihelp.cpp b/src/mapihelp.cpp
index 6d2c04f..6956e43 100644
--- a/src/mapihelp.cpp
+++ b/src/mapihelp.cpp
@@ -459,7 +459,7 @@ mapi_get_header (LPMESSAGE message)
   if (!message)
     return ret;
 
-  hr = message->OpenProperty (PR_TRANSPORT_MESSAGE_HEADERS_A, &IID_IStream, 0, 0,
+  hr = gpgol_openProperty (message, PR_TRANSPORT_MESSAGE_HEADERS_A, &IID_IStream, 0, 0,
                               (LPUNKNOWN*)&stream);
   if (hr)
     {
@@ -501,7 +501,7 @@ mapi_get_body_as_stream (LPMESSAGE message)
       /* The store knows about the Internet Charset Body property,
          thus try to get the body from this property if it exists.  */
 
-      hr = message->OpenProperty (tag, &IID_IStream, 0, 0, 
+      hr = gpgol_openProperty (message, tag, &IID_IStream, 0, 0,
                                   (LPUNKNOWN*)&stream);
       if (!hr)
         return stream;
@@ -514,7 +514,7 @@ mapi_get_body_as_stream (LPMESSAGE message)
      need to implement some kind of stream filter to translated to
      utf-8 or read everyting into a memory buffer and [provide an
      istream from that memory buffer.  */
-  hr = message->OpenProperty (PR_BODY_A, &IID_IStream, 0, 0, 
+  hr = gpgol_openProperty (message, PR_BODY_A, &IID_IStream, 0, 0,
                               (LPUNKNOWN*)&stream);
   if (hr)
     {
@@ -567,7 +567,7 @@ mapi_get_body (LPMESSAGE message, size_t *r_nbytes)
     }
   else /* Message is large; use an IStream to read it.  */
     {
-      hr = message->OpenProperty (PR_BODY, &IID_IStream, 0, 0, 
+      hr = gpgol_openProperty (message, PR_BODY, &IID_IStream, 0, 0,
                                   (LPUNKNOWN*)&stream);
       if (hr)
         {
@@ -656,7 +656,7 @@ get_msgcls_from_pgp_lines (LPMESSAGE message, bool *r_nobody = nullptr)
     {
       log_debug ("%s:%s: Failed to get body ASCII stream.",
                  SRCNAME, __func__);
-      hr = message->OpenProperty (PR_BODY_W, &IID_IStream, 0, 0,
+      hr = gpgol_openProperty (message, PR_BODY_W, &IID_IStream, 0, 0,
                                   (LPUNKNOWN*)&stream);
       if (hr)
         {
@@ -856,7 +856,7 @@ is_really_cms_encrypted (LPMESSAGE message)
       goto leave;
     }
   
-  hr = att->OpenProperty (PR_ATTACH_DATA_BIN, &IID_IStream, 
+  hr = gpgol_openProperty (att, PR_ATTACH_DATA_BIN, &IID_IStream,
                           0, 0, (LPUNKNOWN*) &stream);
   if (FAILED (hr))
     {
@@ -2296,6 +2296,7 @@ mapi_create_attach_table (LPMESSAGE message, int fast)
 
   /* Open the attachment table.  */
   hr = message->GetAttachmentTable (0, &mapitable);
+  memdbg_addRef (mapitable);
   if (FAILED (hr))
     {
       log_debug ("%s:%s: GetAttachmentTable failed: hr=%#lx",
@@ -2344,7 +2345,8 @@ mapi_create_attach_table (LPMESSAGE message, int fast)
       table[pos].mapipos = mapirows->aRow[pos].lpProps[0].Value.l;
 
       hr = message->OpenAttach (table[pos].mapipos, NULL,
-                                MAPI_BEST_ACCESS, &att);	
+                                MAPI_BEST_ACCESS, &att);
+      memdbg_addRef (att);
       if (FAILED (hr))
         {
           log_error ("%s:%s: can't open attachment %d (%d): hr=%#lx",
@@ -2455,7 +2457,7 @@ mapi_get_attach_as_stream (LPMESSAGE message, mapi_attach_item_t *item,
       return NULL;
     }
 
-  hr = att->OpenProperty (PR_ATTACH_DATA_BIN, &IID_IStream, 
+  hr = gpgol_openProperty (att, PR_ATTACH_DATA_BIN, &IID_IStream,
                           0, 0, (LPUNKNOWN*) &stream);
   if (FAILED (hr))
     {
@@ -2489,7 +2491,7 @@ attach_to_buffer (LPATTACH att, size_t *r_nbytes)
   ULONG nread;
   char *buffer;
 
-  hr = att->OpenProperty (PR_ATTACH_DATA_BIN, &IID_IStream, 
+  hr = gpgol_openProperty (att, PR_ATTACH_DATA_BIN, &IID_IStream,
                           0, 0, (LPUNKNOWN*) &stream);
   if (FAILED (hr))
     {
@@ -3495,7 +3497,7 @@ mapi_attachment_to_body (LPMESSAGE message, mapi_attach_item_t *item)
       goto leave;
     }
 
-  hr = att->OpenProperty (PR_ATTACH_DATA_BIN, &IID_IStream, 
+  hr = gpgol_openProperty (att, PR_ATTACH_DATA_BIN, &IID_IStream,
                           0, 0, (LPUNKNOWN*) &instream);
   if (FAILED (hr))
     {
@@ -3506,7 +3508,7 @@ mapi_attachment_to_body (LPMESSAGE message, mapi_attach_item_t *item)
 
 
   punk = (LPUNKNOWN)outstream;
-  hr = message->OpenProperty (PR_BODY_A, &IID_IStream, 0,
+  hr = gpgol_openProperty (message, PR_BODY_A, &IID_IStream, 0,
                               MAPI_CREATE|MAPI_MODIFY, &punk);
   if (FAILED (hr))
     {
@@ -3618,7 +3620,7 @@ mapi_body_to_attachment (LPMESSAGE message)
     }
 
   punk = (LPUNKNOWN)outstream;
-  hr = newatt->OpenProperty (PR_ATTACH_DATA_BIN, &IID_IStream, 0,
+  hr = gpgol_openProperty (newatt, PR_ATTACH_DATA_BIN, &IID_IStream, 0,
                              MAPI_CREATE|MAPI_MODIFY, &punk);
   if (FAILED (hr))
     {
@@ -3728,6 +3730,7 @@ mapi_mark_or_create_moss_attach (LPMESSAGE message, msgtype_t msgtype)
                      SRCNAME, __func__, item->mapipos);
           return -1;
         }
+      memdbg_addRef (att);
       if (!mapi_test_attach_hidden (att))
         {
           mapi_set_attach_hidden (att);
diff --git a/src/mimemaker.cpp b/src/mimemaker.cpp
index b1dfc31..29dcb3c 100644
--- a/src/mimemaker.cpp
+++ b/src/mimemaker.cpp
@@ -214,7 +214,7 @@ create_mapi_attachment (LPMESSAGE message, sink_t sink,
     }
 
   punk = NULL;
-  hr = att->OpenProperty(PR_ATTACH_DATA_BIN, &IID_IStream, 0,
+  hr = gpgol_openProperty (att, PR_ATTACH_DATA_BIN, &IID_IStream, 0,
                          (MAPI_CREATE|MAPI_MODIFY), &punk);
   if (FAILED (hr))
     {
diff --git a/src/mlang-charset.cpp b/src/mlang-charset.cpp
index 934c1e3..eb4cc85 100644
--- a/src/mlang-charset.cpp
+++ b/src/mlang-charset.cpp
@@ -53,6 +53,7 @@ char *ansi_charset_to_utf8 (const char *charset, const char *input,
 
   CoCreateInstance(CLSID_CMultiLanguage, NULL, CLSCTX_INPROC_SERVER,
                    IID_IMultiLanguage, (void**)&multilang);
+  memdbg_addRef (multilang);
 
   if (!multilang)
     {
diff --git a/src/olflange.cpp b/src/olflange.cpp
index 3b25340..f1dd3c3 100644
--- a/src/olflange.cpp
+++ b/src/olflange.cpp
@@ -393,6 +393,7 @@ install_forms (void)
                  SRCNAME, __func__);
       return;
     }
+  memdbg_addRef (formcontainer);
 
   datadir = get_data_dir ();
   if (!datadir)
diff --git a/src/oomhelp.cpp b/src/oomhelp.cpp
index 694ef59..2da1703 100644
--- a/src/oomhelp.cpp
+++ b/src/oomhelp.cpp
@@ -44,6 +44,22 @@ gpgol_queryInterface (LPUNKNOWN pObj, REFIID riid, LPVOID FAR *ppvObj)
   return ret;
 }
 
+HRESULT
+gpgol_openProperty (LPMAPIPROP obj, ULONG ulPropTag, LPCIID lpiid,
+                    ULONG ulInterfaceOptions, ULONG ulFlags,
+                    LPUNKNOWN FAR * lppUnk)
+{
+  HRESULT ret = obj->OpenProperty (ulPropTag, lpiid,
+                                   ulInterfaceOptions, ulFlags,
+                                   lppUnk);
+  if ((opt.enable_debug & DBG_OOM_EXTRA) && *lppUnk)
+    {
+      memdbg_addRef (*lppUnk);
+      log_debug ("%s:%s: OpenProperty on %p prop %lx result %p",
+                 SRCNAME, __func__, obj,  ulPropTag, *lppUnk);
+    }
+  return ret;
+}
 /* Return a malloced string with the utf-8 encoded name of the object
    or NULL if not available.  */
 char *
@@ -712,7 +728,10 @@ get_oom_iunknown (LPDISPATCH pDisp, const char *name)
         log_debug ("%s:%s: Property `%s' is not of class IUnknown (vt=%d)",
                    SRCNAME, __func__, name, rVariant.vt);
       else
-        return rVariant.punkVal;
+        {
+          memdbg_addRef (rVariant.punkVal);
+          return rVariant.punkVal;
+        }
 
       VariantClear (&rVariant);
     }
@@ -1555,6 +1574,7 @@ get_oom_base_message_from_mapi (LPDISPATCH mapi_message)
   log_oom_extra("%s:%s: About to call GetBaseMessage.",
                 SRCNAME, __func__);
   hr = secureMessage->GetBaseMessage (&message);
+  memdbg_addRef (message);
   gpgol_release (secureMessage);
   if (hr != S_OK)
     {
diff --git a/src/oomhelp.h b/src/oomhelp.h
index 1991bee..3a94579 100644
--- a/src/oomhelp.h
+++ b/src/oomhelp.h
@@ -372,4 +372,8 @@ char *get_sender_SendUsingAccount (LPDISPATCH mailitem, bool *r_is_GSuite);
 
 /* memtracing query interface */
 HRESULT gpgol_queryInterface (LPUNKNOWN pObj, REFIID riid, LPVOID FAR *ppvObj);
+
+HRESULT gpgol_openProperty (LPMAPIPROP obj, ULONG ulPropTag, LPCIID lpiid,
+                            ULONG ulInterfaceOptions, ULONG ulFlags,
+                            LPUNKNOWN FAR * lppUnk);
 #endif /*OOMHELP_H*/
diff --git a/src/ribbon-callbacks.cpp b/src/ribbon-callbacks.cpp
index e31d208..9ba0796 100644
--- a/src/ribbon-callbacks.cpp
+++ b/src/ribbon-callbacks.cpp
@@ -134,8 +134,9 @@ getIcon (int id, VARIANT* result)
           IStream* pStream = NULL;
           CopyMemory (pBuffer, pResourceData, imageSize);
 
-          if (CreateStreamOnHGlobal (hBuffer, FALSE, &pStream) == S_OK)
+          if (CreateStreamOnHGlobal (hBuffer, TRUE, &pStream) == S_OK)
             {
+              memdbg_addRef (pStream);
               pbitmap = Gdiplus::Bitmap::FromStream (pStream);
               gpgol_release (pStream);
               if (!pbitmap || pbitmap->GetHBITMAP (0, &pdesc.bmp.hbitmap))

commit 5959ec68f8ae30e3085d5a180db39d7ca4d48817
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jul 20 10:30:23 2018 +0200

    Improve GpgolAddin debug output
    
    * src/gpgoladdin.cpp (DllCanUnloadNow): Trace call.
    (GpgolRibbonExtender::~GpgolRibbonExtender): Dump memory map.
    (GpgolAddinFactory::~GpgolAddinFactory): Debug dtor.

diff --git a/src/gpgoladdin.cpp b/src/gpgoladdin.cpp
index 220602b..0a0fe10 100644
--- a/src/gpgoladdin.cpp
+++ b/src/gpgoladdin.cpp
@@ -103,6 +103,7 @@ STDAPI DllCanUnloadNow()
      unload the Library. Any callbacks will become invalid.
      So we _only_ say it's ok to unload if we were disconnected.
      For the epic story behind the next line see GnuPG-Bug-Id 1837 */
+  TRACEPOINT;
   return can_unload ? S_OK : S_FALSE;
 }
 
@@ -154,6 +155,11 @@ STDMETHODIMP GpgolAddinFactory::CreateInstance (LPUNKNOWN punk, REFIID riid,
   return hr;
 }
 
+GpgolAddinFactory::~GpgolAddinFactory()
+{
+  log_debug ("%s:%s: Object deleted\n", SRCNAME, __func__);
+}
+
 /* GpgolAddin definition */
 
 
@@ -577,7 +583,7 @@ GpgolRibbonExtender::~GpgolRibbonExtender (void)
 {
   log_debug ("%s:%s: cleaning up GpgolRibbonExtender object;",
              SRCNAME, __func__);
-  log_debug ("%s:%s: Object deleted\n", SRCNAME, __func__);
+  memdbg_dump ();
 }
 
 STDMETHODIMP
diff --git a/src/gpgoladdin.h b/src/gpgoladdin.h
index 05c23c0..e2ae977 100644
--- a/src/gpgoladdin.h
+++ b/src/gpgoladdin.h
@@ -232,7 +232,7 @@ class GpgolAddinFactory: public IClassFactory
 {
 public:
   GpgolAddinFactory(): m_lRef(0){}
-  virtual ~GpgolAddinFactory(){}
+  virtual ~GpgolAddinFactory();
 
   STDMETHODIMP QueryInterface (REFIID riid, LPVOID* ppvObj);
   inline STDMETHODIMP_(ULONG) AddRef() { ++m_lRef;  return m_lRef; };

commit af4626b1fdcebbf9fe36d4aae9c664df7780099d
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jul 20 10:28:42 2018 +0200

    Don't export DllCanUnloadNow at all
    
    * src/gpgol.def (DllCanUnloadNow): Don't export.
    
    --
    We don't want to be unloaded by com cleanup. And
    while we should have returned false always I
    saw one crash immediately after a Dll can unload
    now.

diff --git a/src/gpgol.def b/src/gpgol.def
index 40ba133..603716e 100644
--- a/src/gpgol.def
+++ b/src/gpgol.def
@@ -5,6 +5,5 @@ EXPORTS
     DllRegisterServer = DllRegisterServer at 0      @1	PRIVATE
     DllUnregisterServer = DllUnregisterServer at 0  @2	PRIVATE
     DllGetClassObject = DllGetClassObject at 12     @3 PRIVATE
-    DllCanUnloadNow = DllCanUnloadNow at 0          @4 PRIVATE
 
     gpgol_check_version = gpgol_check_version at 4     @11

commit 2e63ea41583fc7cfb3f7e1442f6d9c6a846d6d16
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jul 20 10:27:26 2018 +0200

    Also debug oom if debug oom_extra is set
    
    * src/common_indep.h (gpgol_release): Add line.
    (debug_oom): Also debug for oom_extra
    
    --
    Debug oom is one of the debug settings that need to go
    in the future.

diff --git a/src/common_indep.h b/src/common_indep.h
index 9261d5f..0997594 100644
--- a/src/common_indep.h
+++ b/src/common_indep.h
@@ -258,7 +258,8 @@ char *trim_trailing_spaces (char *string);
               while(*_vptr) { *_vptr=0; _vptr++; } \
                   } while(0)
 
-#define debug_oom        (opt.enable_debug & DBG_OOM)
+#define debug_oom        ((opt.enable_debug & DBG_OOM) || \
+                          (opt.enable_debug & DBG_OOM_EXTRA))
 #define debug_oom_extra  (opt.enable_debug & DBG_OOM_EXTRA)
 void log_debug (const char *fmt, ...) __attribute__ ((format (printf,1,2)));
 void log_error (const char *fmt, ...) __attribute__ ((format (printf,1,2)));
@@ -279,8 +280,8 @@ void log_hexdump (const void *buf, size_t buflen, const char *fmt,
 { \
   if (X && opt.enable_debug & DBG_OOM_EXTRA) \
     { \
-      log_debug ("%s:%s: Object: %p released ref: %lu \n", \
-                 SRCNAME, __func__, X, X->Release()); \
+      log_debug ("%s:%s:%i: Object: %p released ref: %lu \n", \
+                 SRCNAME, __func__, __LINE__, X, X->Release()); \
       memdbg_released (X); \
     } \
   else if (X) \

commit 74c6f5d78ee6bf31fc70ef5ad0b6cd3b3656a0aa
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jul 20 10:26:48 2018 +0200

    Decorate eventsink with memdbg helpers
    
    * src/eventsink.h: Add memdbg helpers

diff --git a/src/eventsink.h b/src/eventsink.h
index 5468c41..12b1b35 100644
--- a/src/eventsink.h
+++ b/src/eventsink.h
@@ -176,6 +176,7 @@ LPDISPATCH install_ ## subcls ## _sink (LPDISPATCH object)               \
   pCPC = (LPCONNECTIONPOINTCONTAINER)disp;                               \
   pCP = NULL;                                                            \
   hr = pCPC->FindConnectionPoint (iidcls, &pCP);                         \
+  memdbg_addRef (pCP);                                                   \
   if (hr != S_OK || !pCP)                                                \
     {                                                                    \
       log_error ("%s:%s:%s: ConnectionPoint not found: hr=%#lx",         \
@@ -185,6 +186,7 @@ LPDISPATCH install_ ## subcls ## _sink (LPDISPATCH object)               \
     }                                                                    \
   sink = new subcls; /* Note: Advise does another AddRef.  */            \
   hr = pCP->Advise ((LPUNKNOWN)sink, &cookie);                           \
+  memdbg_addRef (sink);                                                  \
   gpgol_release (pCPC);                                                      \
   if (hr != S_OK)                                                        \
     {                                                                    \
@@ -210,7 +212,7 @@ void detach_ ## subcls ## _sink (LPDISPATCH obj)                         \
                                                                          \
   if (debug_oom_extra)                                                   \
     log_debug ("%s:%s:%s: Called", SRCNAME, #subcls, __func__);          \
-  hr = obj->QueryInterface (iidcls, (void**)&sink);                      \
+  hr = gpgol_queryInterface (obj, iidcls, (void**)&sink);                \
   if (hr != S_OK || !sink)                                               \
     {                                                                    \
       log_error ("%s:%s:%s: invalid object passed: hr=%#lx",             \

commit fe5ce6b8faf4503f8996f4bd10afdaa8b9aaba0c
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jul 20 08:28:58 2018 +0200

    Fix minor ref leak
    
    * src/oomhelp.cpp (get_ol_ui_language): Release langsettings.

diff --git a/src/oomhelp.cpp b/src/oomhelp.cpp
index 7bf70eb..694ef59 100644
--- a/src/oomhelp.cpp
+++ b/src/oomhelp.cpp
@@ -2269,9 +2269,11 @@ get_ol_ui_language ()
   dispparams.cArgs = 1;
   dispparams.cNamedArgs = 0;
 
-  if (invoke_oom_method_with_parms_type (langSettings, "LanguageID", &var,
-                                         &dispparams,
-                                         DISPATCH_PROPERTYGET))
+  int ret = invoke_oom_method_with_parms_type (langSettings, "LanguageID", &var,
+                                               &dispparams,
+                                               DISPATCH_PROPERTYGET);
+  gpgol_release (langSettings);
+  if (ret)
     {
       TRACEPOINT;
       return 0;

commit 05c70803c61420cc61f566b577117522ead3f1a4
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Fri Jul 20 08:20:18 2018 +0200

    Start memdbg interface for COM ref counting
    
    * src/Makefile.am: Add memdbg files.
    * src/common_indep.h (gpgol_release): Call memdbg_released.
    * src/eventsink.h (install..sink): Use gpgol_queryInterface
    * src/mailitem-events.cpp (Unload): Dump memdbg maps after unload.
    * src/oomhelp.h (gpgol_queryInterface): New.
    * src/mimedataprovider.cpp (MimeDataProvider::MimeDataProvider):
    Register ref add.
    * src/oomhelp.cpp (gpgol_queryInterface): New.
    (get_object_name): Use non memdbg aware functions.
    (get_oom_object): Use gpgol_queryInterface.
    (get_oom_object): Print out refs when obtaining if.
    (get_oom_control_bytag, get_object_by_id),
    (get_oom_mapi_session): Use mdbg functions.
    
    --
    The idea is that in the end we will trace all
    com reference obtains and releases to find mismatchs
    and additionally all malloc / frees and C++ dtor / ctor
    to find memory leaks and hopefully random instability or
    crashes caused by invalid ref counting.
    
    As we can't use valgrind or decent debugging in
    Outlook on Windows tracking it ourself seems
    to be useful.

diff --git a/src/Makefile.am b/src/Makefile.am
index d3d8b19..599394f 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -51,6 +51,7 @@ gpgol_SOURCES = \
     mailitem-events.cpp \
     main.c \
     mapihelp.cpp mapihelp.h \
+    memdbg.cpp memdbg.h \
     mimedataprovider.cpp mimedataprovider.h \
     mimemaker.cpp mimemaker.h \
     mlang-charset.cpp mlang-charset.h \
diff --git a/src/common_indep.h b/src/common_indep.h
index 00921e6..9261d5f 100644
--- a/src/common_indep.h
+++ b/src/common_indep.h
@@ -30,6 +30,8 @@
 
 #include "xmalloc.h"
 
+#include "memdbg.h"
+
 #ifdef HAVE_W32_SYSTEM
 /* Not so independenent ;-) need this for logging HANDLE */
 # include <windows.h>
@@ -279,6 +281,7 @@ void log_hexdump (const void *buf, size_t buflen, const char *fmt,
     { \
       log_debug ("%s:%s: Object: %p released ref: %lu \n", \
                  SRCNAME, __func__, X, X->Release()); \
+      memdbg_released (X); \
     } \
   else if (X) \
     { \
diff --git a/src/eventsink.h b/src/eventsink.h
index 4d81dfe..5468c41 100644
--- a/src/eventsink.h
+++ b/src/eventsink.h
@@ -165,8 +165,8 @@ LPDISPATCH install_ ## subcls ## _sink (LPDISPATCH object)               \
   DWORD cookie;                                                          \
                                                                          \
   disp = NULL;                                                           \
-  hr = object->QueryInterface (IID_IConnectionPointContainer,            \
-                               (LPVOID*)&disp);                          \
+  hr = gpgol_queryInterface (object, IID_IConnectionPointContainer,      \
+                             (LPVOID*)&disp);                            \
   if (hr != S_OK || !disp)                                               \
     {                                                                    \
       log_error ("%s:%s:%s: IConnectionPoint not supported: hr=%#lx",    \
@@ -199,6 +199,7 @@ LPDISPATCH install_ ## subcls ## _sink (LPDISPATCH object)               \
   sink->m_cookie = cookie;                                               \
   sink->m_pCP = pCP;                                                     \
   object->AddRef ();                                                     \
+  memdbg_addRef (object);                                                \
   sink->m_object = object;                                               \
   return (LPDISPATCH)sink;                                               \
 }                                                                        \
diff --git a/src/mailitem-events.cpp b/src/mailitem-events.cpp
index ab100c5..0769df1 100644
--- a/src/mailitem-events.cpp
+++ b/src/mailitem-events.cpp
@@ -727,6 +727,7 @@ EVENT_SINK_INVOKE(MailItemEvents)
           delete m_mail;
           log_oom_extra ("%s:%s: deletion done",
                          SRCNAME, __func__);
+          memdbg_dump ();
           return S_OK;
         }
       /* Fallthrough */
diff --git a/src/memdbg.cpp b/src/memdbg.cpp
new file mode 100644
index 0000000..cd506dc
--- /dev/null
+++ b/src/memdbg.cpp
@@ -0,0 +1,174 @@
+/* @file memdbg.cpp
+ * @brief Memory debugging helpers
+ *
+ * Copyright (C) 2018 Intevation GmbH
+ *
+ * This file is part of GpgOL.
+ *
+ * GpgOL is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * GpgOL is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "config.h"
+
+#include "memdbg.h"
+
+#include "common_indep.h"
+
+#include <gpg-error.h>
+
+#include <map>
+#include <string>
+
+std::map <std::string, int> cppObjs;
+std::map <void *, int> olObjs;
+std::map <void *, std::string> olNames;
+
+GPGRT_LOCK_DEFINE (memdbg_log);
+
+#define DBGGUARD if (!(opt.enable_debug & DBG_OOM_EXTRA)) return
+
+#ifdef HAVE_W32_SYSTEM
+# include "oomhelp.h"
+#endif
+
+static void
+register_name (void *obj)
+{
+#ifdef HAVE_W32_SYSTEM
+  auto it = olNames.find (obj);
+
+  if (it != olNames.end())
+    {
+      return;
+    }
+
+  char *name = get_object_name ((LPUNKNOWN)obj);
+  if (!name)
+    {
+      return;
+    }
+  std::string sName = name;
+  olNames.insert (std::make_pair (obj, sName));
+  xfree (name);
+#else
+  (void) obj;
+#endif
+}
+
+void
+memdbg_addRef (void *obj)
+{
+  DBGGUARD;
+
+  if (!obj)
+    {
+      return;
+    }
+
+  gpgrt_lock_lock (&memdbg_log);
+
+  auto it = olObjs.find (obj);
+
+  if (it == olObjs.end())
+    {
+      it = olObjs.insert (std::make_pair (obj, 0)).first;
+      register_name (obj);
+    }
+  it->second++;
+
+  gpgrt_lock_unlock (&memdbg_log);
+}
+
+void
+memdbg_released (void *obj)
+{
+  DBGGUARD;
+
+  if (!obj)
+    {
+      return;
+    }
+
+  gpgrt_lock_lock (&memdbg_log);
+
+  auto it = olObjs.find (obj);
+
+  if (it == olObjs.end())
+    {
+      log_error ("%s:%s Released %p without query if!!",
+                 SRCNAME, __func__, obj);
+      gpgrt_lock_unlock (&memdbg_log);
+      return;
+    }
+
+  it->second--;
+
+  if (it->second < 0)
+    {
+      log_error ("%s:%s Released %p below zero",
+                 SRCNAME, __func__, obj);
+    }
+  gpgrt_lock_unlock (&memdbg_log);
+}
+
+void
+memdbg_ctor (const char *objName)
+{
+  (void)objName;
+}
+
+void
+memdbg_dtor (const char *objName)
+{
+  (void)objName;
+
+}
+
+void
+memdbg_dump ()
+{
+  gpgrt_lock_lock (&memdbg_log);
+  log_debug(""
+"------------------------------MEMORY DUMP----------------------------------");
+
+  log_debug("-- C++ Objects --");
+  for (const auto &pair: cppObjs)
+    {
+      log_debug("%s\t: %i", pair.first.c_str(), pair.second);
+    }
+  log_debug("-- C++ End --");
+  log_debug("-- OL Objects --");
+  for (const auto &pair: olObjs)
+    {
+      if (!pair.second)
+        {
+          continue;
+        }
+      const auto it = olNames.find (pair.first);
+      if (it == olNames.end())
+        {
+          log_debug("%p\t: %i", pair.first, pair.second);
+        }
+      else
+        {
+          log_debug("%p:%s\t: %i", pair.first,
+                    it->second.c_str (), pair.second);
+        }
+    }
+  log_debug("-- OL End --");
+
+  log_debug(""
+"------------------------------MEMORY END ----------------------------------");
+  gpgrt_lock_unlock (&memdbg_log);
+}
diff --git a/src/memdbg.h b/src/memdbg.h
new file mode 100644
index 0000000..a9ea4c5
--- /dev/null
+++ b/src/memdbg.h
@@ -0,0 +1,44 @@
+#ifndef MEMDBG_H
+#define MEMDBG_H
+
+/* @file memdbg.h
+ * @brief Memory debugging helpers
+ *
+ * Copyright (C) 2018 Intevation GmbH
+ *
+ * This file is part of GpgOL.
+ *
+ * GpgOL is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * GpgOL is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#if 0
+}
+#endif
+#endif
+
+void memdbg_addRef (void *obj);
+void memdbg_released (void *obj);
+
+void memdbg_ctor (const char *objName);
+void memdbg_dtor (const char *objName);
+
+void memdbg_dump(void);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif //MEMDBG_H
diff --git a/src/mimedataprovider.cpp b/src/mimedataprovider.cpp
index 92be963..2899f43 100644
--- a/src/mimedataprovider.cpp
+++ b/src/mimedataprovider.cpp
@@ -501,6 +501,7 @@ MimeDataProvider::MimeDataProvider(LPSTREAM stream, bool no_headers):
   if (stream)
     {
       stream->AddRef ();
+      memdbg_addRef (stream);
     }
   else
     {
diff --git a/src/oomhelp.cpp b/src/oomhelp.cpp
index a5822b5..7bf70eb 100644
--- a/src/oomhelp.cpp
+++ b/src/oomhelp.cpp
@@ -33,6 +33,16 @@
 #include "oomhelp.h"
 #include "gpgoladdin.h"
 
+HRESULT
+gpgol_queryInterface (LPUNKNOWN pObj, REFIID riid, LPVOID FAR *ppvObj)
+{
+  HRESULT ret = pObj->QueryInterface (riid, ppvObj);
+  if ((opt.enable_debug & DBG_OOM_EXTRA) && *ppvObj)
+    {
+      memdbg_addRef (*ppvObj);
+    }
+  return ret;
+}
 
 /* Return a malloced string with the utf-8 encoded name of the object
    or NULL if not available.  */
@@ -48,6 +58,7 @@ get_object_name (LPUNKNOWN obj)
   if (!obj)
     goto leave;
 
+  /* We can't use gpgol_queryInterface here to avoid recursion */
   obj->QueryInterface (IID_IDispatch, (void **)&disp);
   if (!disp)
     goto leave;
@@ -73,9 +84,9 @@ get_object_name (LPUNKNOWN obj)
 
  leave:
   if (tinfo)
-    gpgol_release (tinfo);
+    tinfo->Release ();
   if (disp)
-    gpgol_release (disp);
+    disp->Release ();
 
   return name;
 }
@@ -188,7 +199,7 @@ get_oom_object (LPDISPATCH pStart, const char *fullname)
           gpgol_release (pDisp);
           pDisp = NULL;
         }
-      if (pObj->QueryInterface (IID_IDispatch, (LPVOID*)&pDisp) != S_OK)
+      if (gpgol_queryInterface (pObj, IID_IDispatch, (LPVOID*)&pDisp) != S_OK)
         {
           log_error ("%s:%s Object does not support IDispatch",
                      SRCNAME, __func__);
@@ -203,7 +214,13 @@ get_oom_object (LPDISPATCH pStart, const char *fullname)
         return NULL;  /* The object has no IDispatch interface.  */
       if (!*fullname)
         {
-          log_oom ("%s:%s:         got %p",SRCNAME, __func__, pDisp);
+          if ((opt.enable_debug & DBG_OOM))
+            {
+              pDisp->AddRef ();
+              int ref = pDisp->Release ();
+              log_oom ("%s:%s:         got %p with %i refs",
+                       SRCNAME, __func__, pDisp, ref);
+            }
           return pDisp; /* Ready.  */
         }
       
@@ -327,6 +344,7 @@ get_oom_object (LPDISPATCH pStart, const char *fullname)
         }
 
       pObj = vtResult.pdispVal;
+      memdbg_addRef (pObj);
     }
   log_debug ("%s:%s: no object", SRCNAME, __func__);
   return NULL;
@@ -753,7 +771,8 @@ get_oom_control_bytag (LPDISPATCH pDisp, const char *tag)
   SysFreeString (bstring);
   if (hr == S_OK && rVariant.vt == VT_DISPATCH && rVariant.pdispVal)
     {
-      rVariant.pdispVal->QueryInterface (IID_IDispatch, (LPVOID*)&result);
+      gpgol_queryInterface (rVariant.pdispVal, IID_IDispatch,
+                            (LPVOID*)&result);
       gpgol_release (rVariant.pdispVal);
       if (!result)
         log_debug ("%s:%s: Object with tag `%s' has no dispatch intf.",
@@ -1459,7 +1478,7 @@ get_object_by_id (LPDISPATCH pDisp, REFIID id)
   if (!pDisp)
     return NULL;
 
-  if (pDisp->QueryInterface (id, (void **)&disp) != S_OK)
+  if (gpgol_queryInterface(pDisp, id, (void **)&disp) != S_OK)
     return NULL;
   return disp;
 }
@@ -1636,7 +1655,7 @@ get_oom_mapi_session ()
       return NULL;
     }
   session = NULL;
-  hr = mapiobj->QueryInterface (IID_IMAPISession, (void**)&session);
+  hr = gpgol_queryInterface (mapiobj, IID_IMAPISession, (void**)&session);
   gpgol_release (mapiobj);
   if (hr != S_OK || !session)
     {
diff --git a/src/oomhelp.h b/src/oomhelp.h
index 4430b0c..1991bee 100644
--- a/src/oomhelp.h
+++ b/src/oomhelp.h
@@ -369,4 +369,7 @@ int get_ex_major_version_for_addr (const char *mbox);
 int get_ol_ui_language (void);
 
 char *get_sender_SendUsingAccount (LPDISPATCH mailitem, bool *r_is_GSuite);
+
+/* memtracing query interface */
+HRESULT gpgol_queryInterface (LPUNKNOWN pObj, REFIID riid, LPVOID FAR *ppvObj);
 #endif /*OOMHELP_H*/

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

Summary of changes:
 src/Makefile.am             |   1 +
 src/common_indep.h          |  10 ++-
 src/eventsink.h             |   9 +-
 src/gpgol.def               |   1 -
 src/gpgoladdin.cpp          |  18 +++-
 src/gpgoladdin.h            |   2 +-
 src/mail.cpp                |  13 +--
 src/mailitem-events.cpp     |   1 +
 src/mapihelp.cpp            |  27 +++---
 src/memdbg.cpp              | 203 ++++++++++++++++++++++++++++++++++++++++++++
 src/{xmalloc.h => memdbg.h} |  34 ++++----
 src/mimedataprovider.cpp    |   1 +
 src/mimemaker.cpp           |   2 +-
 src/mlang-charset.cpp       |   1 +
 src/olflange.cpp            |   1 +
 src/oomhelp.cpp             |  63 +++++++++++---
 src/oomhelp.h               |   7 ++
 src/ribbon-callbacks.cpp    |   3 +-
 18 files changed, 341 insertions(+), 56 deletions(-)
 create mode 100644 src/memdbg.cpp
 copy src/{xmalloc.h => memdbg.h} (69%)


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




More information about the Gnupg-commits mailing list