[Top][All Lists]

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

Re: [PATCH] relocatable: free memory in DLL_PROCESS_DETACH

From: Bruno Haible
Subject: Re: [PATCH] relocatable: free memory in DLL_PROCESS_DETACH
Date: Sun, 08 Aug 2021 19:18:51 +0200
User-agent: KMail/5.1.3 (Linux/4.4.0-210-generic; KDE/5.18.0; x86_64; ; )


Jonathan Boeing wrote:
> Claws Mail uses the Enchant spell checking library, which we build with
> relocatable support for Windows.
> Running with app verifier enabled caught a memory leak in
> lib/relocatable.c.  The memory allocated in DllMain() by:
> shared_library_fullname = strdup (location);
> is never freed.  _DLL_InitTerm() looks like it has the same issue, and
> find_shared_library_fullname() seems to have a similar issue with the
> call to getline().
> This is noticeable in Claws because we load/unload Enchant when we
> open/close a message compose window.

Thanks for explaining the use-case. I could not imagine why it would
be useful to care about DLL_PROCESS_DETACH, i.e. why programs would
repeatedly Load and Unload a library. Now I can imagine it.

> This patch addresses the Windows case by freeing the memory in

This is not correct, because the code that calls
get_shared_library_fullname ()
would then use a pointer to memory that has already been freed,
and thus crash.

Instead, I'm applying a patch that avoid allocating more memory
in case of a second DLL_PROCESS_ATTACH.

> It also initializes shared_library_fullname to NULL
> so that it doesn't try to free an uninitialized pointer in the
> unexpected situation of something failing in DLL_PROCESS_ATTACH.

In C, 'static' storage is always zero-initialized. Do you have
indications that this does not hold for storage that is allocated
as part of a DLL? If so, please, can you provide a pointer to the
documentation of it, on microsoft.com?

2021-08-08  Bruno Haible  <bruno@clisp.org>

        relocatable-lib-lgpl: Fix a memory leak related to a Windows DLL.
        Reported by Jonathan Boeing <jonathan@claws-mail.org> in
        * lib/relocatable.c (DllMain): Avoid memory leak in a special case
        of repeated attach/detach.

diff --git a/lib/relocatable.c b/lib/relocatable.c
index ababc35..a682912 100644
--- a/lib/relocatable.c
+++ b/lib/relocatable.c
@@ -323,7 +323,10 @@ static char *shared_library_fullname;
    supports longer file names
    (see <https://cygwin.com/ml/cygwin/2011-01/msg00410.html>).  */
-/* Determine the full pathname of the shared library when it is loaded.  */
+/* Determine the full pathname of the shared library when it is loaded.
+   Documentation:
+   <https://docs.microsoft.com/en-us/windows/win32/dlls/dllmain>  */
 DllMain (HINSTANCE module_handle, DWORD event, LPVOID reserved)
@@ -343,7 +346,13 @@ DllMain (HINSTANCE module_handle, DWORD event, LPVOID 
         /* Shouldn't happen.  */
         return FALSE;
-      shared_library_fullname = strdup (location);
+      /* Avoid a memory leak when the same DLL get attached, detached,
+         attached, detached, and so on.  This happens e.g. when a spell
+         checker DLL is used repeatedly by a mail program.  */
+      if (!(shared_library_fullname != NULL
+            && strcmp (shared_library_fullname, location) == 0))
+        /* Remember the full pathname of the shared library.  */
+        shared_library_fullname = strdup (location);
   return TRUE;

reply via email to

[Prev in Thread] Current Thread [Next in Thread]