* elf/dl-close.c (_dl_close_worker): Renamed from _dl_close and

	split out locking and parameter checking.
	(_dl_close): Call _dl_close_worker after locking and checking.
	* elf/dl-open.c (_dl_open): Call _dl_close_worker instead of
	_dl_close.
	* elf/Makefile: Add rules to build and run tst-thrlock.
	* elf/tst-thrlock.c:  New file.

	[BZ #3429]
	* elf/dl-open.c (dl_open_worker): Keep holding dl_load_lock until
	we are sure we do not need it anymore for _dl_close.  Also move
	the asserts inside the lock region.
	Patch mostly by Suzuki <suzuki@in.ibm.com>.

4 files changed, 44 insertions, 35 deletions

diff --git a/elf/Makefile b/elf/Makefile
index b1a9bab022..06e376d803 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -171,7 +171,7 @@ tests += loadtest restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-dlmopen1 tst-dlmopen2 tst-dlmopen3 \
 	 unload3 unload4 unload5 unload6 unload7 tst-global1 order2 \
 	 tst-audit1 tst-audit2 \
-	 tst-stackguard1 tst-addr1
+	 tst-stackguard1 tst-addr1 tst-thrlock
 #	 reldep9
 test-srcs = tst-pathopt
 tests-vis-yes = vismain
@@ -916,3 +916,5 @@ $(objpfx)tst-leaks1-mem: $(objpfx)tst-leaks1.out
 tst-leaks1-ENV = MALLOC_TRACE=$(objpfx)tst-leaks1.mtrace
 
 $(objpfx)tst-addr1: $(libdl)
+
+$(objpfx)tst-thrlock: $(libdl) $(shared-thread-library)
diff --git a/elf/dl-close.c b/elf/dl-close.c
index 6a2ad976a7..cc7a9b3213 100644
--- a/elf/dl-close.c
+++ b/elf/dl-close.c
@@ -101,22 +101,9 @@ remove_slotinfo (size_t idx, struct dtv_slotinfo_list *listp, size_t disp,
 
 
 void
-_dl_close (void *_map)
+_dl_close_worker (struct link_map *map)
 {
-  struct link_map *map = _map;
   Lmid_t ns = map->l_ns;
-  unsigned int i;
-  /* First see whether we can remove the object at all.  */
-  if (__builtin_expect (map->l_flags_1 & DF_1_NODELETE, 0)
-      && map->l_init_called)
-    /* Nope.  Do nothing.  */
-    return;
-
-  if (__builtin_expect (map->l_direct_opencount, 1) == 0)
-    GLRO(dl_signal_error) (0, map->l_name, NULL, N_("shared object not open"));
-
-  /* Acquire the lock.  */
-  __rtld_lock_lock_recursive (GL(dl_load_lock));
 
   /* One less direct use.  */
   --map->l_direct_opencount;
@@ -137,7 +124,6 @@ _dl_close (void *_map)
 	_dl_debug_printf ("\nclosing file=%s; direct_opencount=%u\n",
 			  map->l_name, map->l_direct_opencount);
 
-      __rtld_lock_unlock_recursive (GL(dl_load_lock));
       return;
     }
 
@@ -240,7 +226,7 @@ _dl_close (void *_map)
 #endif
   bool unload_any = false;
   unsigned int first_loaded = ~0;
-  for (i = 0; i < nloaded; ++i)
+  for (unsigned int i = 0; i < nloaded; ++i)
     {
       struct link_map *imap = maps[i];
 
@@ -411,7 +397,7 @@ _dl_close (void *_map)
 
   /* Check each element of the search list to see if all references to
      it are gone.  */
-  for (i = first_loaded; i < nloaded; ++i)
+  for (unsigned int i = first_loaded; i < nloaded; ++i)
     {
       struct link_map *imap = maps[i];
       if (!used[i])
@@ -627,6 +613,30 @@ _dl_close (void *_map)
     goto retry;
 
   dl_close_state = not_pending;
+}
+
+
+void
+_dl_close (void *_map)
+{
+  struct link_map *map = _map;
+
+  /* First see whether we can remove the object at all.  */
+  if (__builtin_expect (map->l_flags_1 & DF_1_NODELETE, 0))
+    {
+      assert (map->l_init_called);
+      /* Nope.  Do nothing.  */
+      return;
+    }
+
+  if (__builtin_expect (map->l_direct_opencount, 1) == 0)
+    GLRO(dl_signal_error) (0, map->l_name, NULL, N_("shared object not open"));
+
+  /* Acquire the lock.  */
+  __rtld_lock_lock_recursive (GL(dl_load_lock));
+
+  _dl_close_worker (map);
+
   __rtld_lock_unlock_recursive (GL(dl_load_lock));
 }
 
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 8d057f82eb..50c0d54f27 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -547,15 +547,9 @@ no more namespaces available for dlmopen()"));
   _dl_unload_cache ();
 #endif
 
-  /* Release the lock.  */
-  __rtld_lock_unlock_recursive (GL(dl_load_lock));
-
+  /* See if an error occurred during loading.  */
   if (__builtin_expect (errstring != NULL, 0))
     {
-      /* Some error occurred during loading.  */
-      char *local_errstring;
-      size_t len_errstring;
-
       /* Remove the object from memory.  It may be in an inconsistent
 	 state if relocation failed, for example.  */
       if (args.map)
@@ -572,12 +566,18 @@ no more namespaces available for dlmopen()"));
 	    GL(dl_tls_dtv_gaps) = true;
 #endif
 
-	  _dl_close (args.map);
+	  _dl_close_worker (args.map);
 	}
 
+      assert (_dl_debug_initialize (0, args.nsid)->r_state == RT_CONSISTENT);
+
+      /* Release the lock.  */
+      __rtld_lock_unlock_recursive (GL(dl_load_lock));
+
       /* Make a local copy of the error string so that we can release the
 	 memory allocated for it.  */
-      len_errstring = strlen (errstring) + 1;
+      size_t len_errstring = strlen (errstring) + 1;
+      char *local_errstring;
       if (objname == errstring + len_errstring)
 	{
 	  size_t total_len = len_errstring + strlen (objname) + 1;
@@ -594,14 +594,15 @@ no more namespaces available for dlmopen()"));
       if (malloced)
 	free ((char *) errstring);
 
-      assert (_dl_debug_initialize (0, args.nsid)->r_state == RT_CONSISTENT);
-
       /* Reraise the error.  */
       _dl_signal_error (errcode, objname, NULL, local_errstring);
     }
 
   assert (_dl_debug_initialize (0, args.nsid)->r_state == RT_CONSISTENT);
 
+  /* Release the lock.  */
+  __rtld_lock_unlock_recursive (GL(dl_load_lock));
+
 #ifndef SHARED
   DL_STATIC_INIT (args.map);
 #endif
diff --git a/elf/tst-thrlock.c b/elf/tst-thrlock.c
index fe72eba141..71f1fbb35d 100644
--- a/elf/tst-thrlock.c
+++ b/elf/tst-thrlock.c
@@ -27,9 +27,8 @@ tf (void *arg)
   return NULL;
 }
 
-
-static int
-do_test (void)
+int
+main (void)
 {
 #define N 10
   pthread_t th[N];
@@ -54,6 +53,3 @@ do_test (void)
     }
   return 0;
 }
-
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"