<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 6, 2015 at 3:51 AM, Pavel Karneliuk <span dir="ltr"><<a href="mailto:pavel.karneliuk@gmail.com" target="_blank">pavel.karneliuk@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi chandlerc, Bigcheese,<br>
<br>
This patch fixes a memory leak in Support library. Currently, the OpenedHandles is allocated dynamically and never deallocated. This patch wraps a raw pointer in llvm::ManagedStatic<> template. So, the registry of open handles will be managed by LLVM routines for static variables (llvm::llvm_shutdown()).<br>
<br>
This patch is close to that <a href="https://github.com/llvm-mirror/llvm/commit/b4b9b3b24c03f414c54f55e1b34b4d9124c55883" target="_blank">https://github.com/llvm-mirror/llvm/commit/b4b9b3b24c03f414c54f55e1b34b4d9124c55883</a> patch.<br>
I have removed fixme-message in the header.<br>
<br>
<a href="http://reviews.llvm.org/D8838" target="_blank">http://reviews.llvm.org/D8838</a><br>
<br>
Files:<br>
lib/Support/DynamicLibrary.cpp<br>
lib/Support/Windows/DynamicLibrary.inc<br>
<br>
Index: lib/Support/DynamicLibrary.cpp<br>
===================================================================<br>
--- lib/Support/DynamicLibrary.cpp<br>
+++ lib/Support/DynamicLibrary.cpp<br>
@@ -9,8 +9,6 @@<br>
//<br>
// This file implements the operating system DynamicLibrary concept.<br>
//<br>
-// FIXME: This file leaks ExplicitSymbols and OpenedHandles!<br>
-//<br>
//===----------------------------------------------------------------------===//<br>
<br>
#include "llvm/Support/DynamicLibrary.h"<br>
@@ -51,7 +49,7 @@<br>
//=== independent code.<br>
//===----------------------------------------------------------------------===//<br>
<br>
-static DenseSet<void *> *OpenedHandles = nullptr;<br>
+static llvm::ManagedStatic<DenseSet<void *> > OpenedHandles;<br></blockquote><div><br>This is one of those "what is a leak" existential questions.<br><br>There are two common attitudes/interpretations:<br><br>1) Memory that is unreachable<br>2) Memory that is never deallocated<br><br>Most tools have been taught to only report (1) and ignore (2).<br><br>This idiom (allocating and never deallocating globals) is sometimes used to avoid global destructors which can be dangerously racy (if a multithreaded program is shutting down (possibly due to exceptions, etc) and hasn't stopped all its worker threads, it could be problematic to try to destroy globals that may still be in use by other threads - leading to failure-on-failure modes that can make debugging the original failure more difficult)<br><br>- David<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
DynamicLibrary DynamicLibrary::getPermanentLibrary(const char *filename,<br>
std::string *errMsg) {<br>
@@ -70,9 +68,6 @@<br>
handle = RTLD_DEFAULT;<br>
#endif<br>
<br>
- if (!OpenedHandles)<br>
- OpenedHandles = new DenseSet<void *>();<br>
-<br>
// If we've already loaded this library, dlclose() the handle in order to<br>
// keep the internal refcount at +1.<br>
if (!OpenedHandles->insert(handle).second)<br>
@@ -121,7 +116,7 @@<br>
<br>
#if HAVE_DLFCN_H<br>
// Now search the libraries.<br>
- if (OpenedHandles) {<br>
+ if (OpenedHandles.isConstructed()) {<br>
for (DenseSet<void *>::iterator I = OpenedHandles->begin(),<br>
E = OpenedHandles->end(); I != E; ++I) {<br>
//lt_ptr ptr = lt_dlsym(*I, symbolName);<br>
Index: lib/Support/Windows/DynamicLibrary.inc<br>
===================================================================<br>
--- lib/Support/Windows/DynamicLibrary.inc<br>
+++ lib/Support/Windows/DynamicLibrary.inc<br>
@@ -39,7 +39,7 @@<br>
//=== and must not be UNIX code.<br>
//===----------------------------------------------------------------------===//<br>
<br>
-static DenseSet<HMODULE> *OpenedHandles;<br>
+static llvm::ManagedStatic<DenseSet<HMODULE> > OpenedHandles;<br>
<br>
static BOOL CALLBACK<br>
ELM_Callback(WIN32_ELMCB_PCSTR ModuleName, ULONG_PTR ModuleBase,<br>
@@ -54,8 +54,6 @@<br>
<br>
if (!filename) {<br>
// When no file is specified, enumerate all DLLs and EXEs in the process.<br>
- if (OpenedHandles == 0)<br>
- OpenedHandles = new DenseSet<HMODULE>();<br>
<br>
EnumerateLoadedModules(GetCurrentProcess(), ELM_Callback, 0);<br>
// Dummy library that represents "search all handles".<br>
@@ -77,9 +75,6 @@<br>
return DynamicLibrary();<br>
}<br>
<br>
- if (OpenedHandles == 0)<br>
- OpenedHandles = new DenseSet<HMODULE>();<br>
-<br>
// If we've already loaded this library, FreeLibrary() the handle in order to<br>
// keep the internal refcount at +1.<br>
if (!OpenedHandles->insert(a_handle).second)<br>
@@ -125,7 +120,7 @@<br>
}<br>
<br>
// Now search the libraries.<br>
- if (OpenedHandles) {<br>
+ if (OpenedHandles.isConstructed()) {<br>
for (DenseSet<HMODULE>::iterator I = OpenedHandles->begin(),<br>
E = OpenedHandles->end(); I != E; ++I) {<br>
FARPROC ptr = GetProcAddress((HMODULE)*I, symbolName);<br>
<br>
EMAIL PREFERENCES<br>
<a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>