https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=33297d810d9033ffca661b8f9158116602615dd7
commit 33297d810d9033ffca661b8f9158116602615dd7
Author: Corinna Vinschen <cori...@vinschen.de>
Date: Tue Mar 21 14:30:24 2017 +0100
Cygwin: dlfcn: Fix reference counting
The original dll_init code was living under the wrong assumption that
dll_dllcrt0_1 and in turn dll_list::alloc will be called for each
LoadLibrary call. The same wrong assumption was made for
cygwin_detach_dll/dll_list::detach called via FreeLibrary.
In reality, dll_dllcrt0_1 gets only called once at first LoadLibrary
and cygwin_detach_dll once at last FreeLibrary.
In effect, reference counting for DLLs was completely broken after fork:
parent:
l1 = dlopen ("lib1"); // LoadLibrary, LoadCount = 1
l2 = dlopen ("lib1"); // LoadLibrary, LoadCount = 2
fork (); // LoadLibrary in the child, LoadCount = 1!
child:
dlclose (l1); // FreeLibrary actually frees the lib
x = dlsym (l2); // SEGV
* Move reference counting to dlopen/dlclose since only those functions
have to keep track of loading/unloading DLLs in the application context.
* Remove broken accounting code from dll_list::alloc and dll_list::detach.
* Fix error handling in dlclose.
Signed-off-by: Corinna Vinschen <cori...@vinschen.de>
Diff:
---
winsup/cygwin/dlfcn.cc | 38 +++++++++++++++++++++++++++--------
winsup/cygwin/dll_init.cc | 49 +++++++++++++++++++++------------------------
winsup/cygwin/release/2.8.0 | 4 ++++
3 files changed, 57 insertions(+), 34 deletions(-)
diff --git a/winsup/cygwin/dlfcn.cc b/winsup/cygwin/dlfcn.cc
index 9959ff7..06e6854 100644
--- a/winsup/cygwin/dlfcn.cc
+++ b/winsup/cygwin/dlfcn.cc
@@ -9,14 +9,15 @@ details. */
#include "winsup.h"
#include <psapi.h>
#include <stdlib.h>
+#include <dlfcn.h>
#include <ctype.h>
#include <wctype.h>
#include "path.h"
#include "fhandler.h"
#include "dtable.h"
+#include "dll_init.h"
#include "cygheap.h"
#include "perprocess.h"
-#include "dlfcn.h"
#include "cygtls.h"
#include "tls_pbuf.h"
#include "ntdll.h"
@@ -291,6 +292,13 @@ dlopen (const char *name, int flags)
#endif
ret = (void *) LoadLibraryW (wpath);
+ /* reference counting */
+ if (ret)
+ {
+ dll *d = dlls.find (ret);
+ if (d)
+ ++d->count;
+ }
#ifndef __x86_64__
/* Restore original cxx_malloc pointer. */
@@ -361,13 +369,27 @@ dlsym (void *handle, const char *name)
extern "C" int
dlclose (void *handle)
{
- int ret;
- if (handle == GetModuleHandle (NULL))
- ret = 0;
- else if (FreeLibrary ((HMODULE) handle))
- ret = 0;
- else
- ret = -1;
+ int ret = 0;
+ if (handle != GetModuleHandle (NULL))
+ {
+ /* reference counting */
+ dll *d = dlls.find (handle);
+ if (d) system_printf ("%W count %d", d->name, d->count);
+ if (!d || d->count <= 0)
+ {
+ errno = ENOENT;
+ ret = -1;
+ }
+ else
+ {
+ --d->count;
+ if (!FreeLibrary ((HMODULE) handle))
+ {
+ __seterrno ();
+ ret = -1;
+ }
+ }
+ }
if (ret)
set_dl_error ("dlclose");
return ret;
diff --git a/winsup/cygwin/dll_init.cc b/winsup/cygwin/dll_init.cc
index 86776f2..490f1d6 100644
--- a/winsup/cygwin/dll_init.cc
+++ b/winsup/cygwin/dll_init.cc
@@ -200,9 +200,8 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type type)
dll *d = (type == DLL_LINK) ? dlls.find_by_modname (modname) : dlls[name];
if (d)
{
- if (!in_forkee)
- d->count++; /* Yes. Bump the usage count. */
- else if (d->handle != h)
+ /* We only get here in the forkee. */
+ if (d->handle != h)
fabort ("%W: Loaded to different address: parent(%p) != child(%p)",
name, d->handle, h);
/* If this DLL has been linked against, and the full path differs, try
@@ -224,15 +223,14 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type
type)
}
else
{
- /* FIXME: Change this to new at some point. */
- d = (dll *) cmalloc (HEAP_2_DLL, sizeof (*d) + (namelen * sizeof
(*name)));
-
+ d = (dll *) cmalloc (HEAP_2_DLL,
+ sizeof (*d) + (namelen * sizeof (*name)));
/* Now we've allocated a block of information. Fill it in with the
supplied info about this DLL. */
- d->count = 1;
wcscpy (d->name, name);
d->modname = d->name + (modname - name);
d->handle = h;
+ d->count = 0; /* Reference counting performed in dlopen/dlclose. */
d->has_dtors = true;
d->p = p;
d->ndeps = 0;
@@ -438,25 +436,21 @@ dll_list::detach (void *retaddr)
guard (true);
if ((d = find (retaddr)))
{
- if (d->count <= 0)
- system_printf ("WARNING: trying to detach an already detached dll ...");
- if (--d->count == 0)
- {
- /* Ensure our exception handler is enabled for destructors */
- exception protect;
- /* Call finalize function if we are not already exiting */
- if (!exit_state)
- __cxa_finalize (d->handle);
- d->run_dtors ();
- d->prev->next = d->next;
- if (d->next)
- d->next->prev = d->prev;
- if (d->type == DLL_LOAD)
- loaded_dlls--;
- if (end == d)
- end = d->prev;
- cfree (d);
- }
+ system_printf ("HERE %W", d->name);
+ /* Ensure our exception handler is enabled for destructors */
+ exception protect;
+ /* Call finalize function if we are not already exiting */
+ if (!exit_state)
+ __cxa_finalize (d->handle);
+ d->run_dtors ();
+ d->prev->next = d->next;
+ if (d->next)
+ d->next->prev = d->prev;
+ if (d->type == DLL_LOAD)
+ loaded_dlls--;
+ if (end == d)
+ end = d->prev;
+ cfree (d);
}
guard (false);
}
@@ -641,6 +635,9 @@ void dll_list::load_after_fork_impl (HANDLE parent, dll* d,
int retries)
if (h != d->handle)
fabort ("unable to map %W to same address as parent: %p != %p",
d->modname, d->handle, h);
+ /* Fix OS reference count. */
+ for (int cnt = 1; cnt < d->count; ++cnt)
+ LoadLibraryW (d->name);
}
}
diff --git a/winsup/cygwin/release/2.8.0 b/winsup/cygwin/release/2.8.0
index 2ccbcf1..92dff74 100644
--- a/winsup/cygwin/release/2.8.0
+++ b/winsup/cygwin/release/2.8.0
@@ -33,3 +33,7 @@ Bug Fixes
- Fix a potential crash in duplocale.
Addresses: https://sourceware.org/ml/newlib/2017/msg00166.html
+
+- Fix dlopen/dlclose reference counting to make sure FreeLibrary isn't
+ called too early in forked process.
+ Addresses: https://cygwin.com/ml/cygwin/2017-03/msg00220.html
