[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; ; ) |
Hi,
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
> DLL_PROCESS_DETACH.
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
<https://lists.gnu.org/archive/html/bug-gnulib/2021-08/msg00048.html>.
* 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> */
BOOL WINAPI
DllMain (HINSTANCE module_handle, DWORD event, LPVOID reserved)
@@ -343,7 +346,13 @@ DllMain (HINSTANCE module_handle, DWORD event, LPVOID
reserved)
/* 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;