Test case for BZ 19329

Message ID 5836CC80.9070101@arm.com
State Superseded
Headers show

Commit Message

Szabolcs Nagy Nov. 24, 2016, 11:18 a.m.
Test concurrent dlopen and pthread_create when the loaded
modules have TLS.  This triggers dl-tls assertion failures
more reliably than the tst-stack4 test.

The dlopened module has 100 DT_NEEDED dependencies and
for me about 4000 concurrent thread creations are needed
to see failures on x86_64.

2016-11-24  Szabolcs Nagy  <szabolcs.nagy@arm.com>

	[BZ #19329]
	* nptl/Makefile (tests): Add tst-tls7.
	(modules-names): Add tst-tls7mod, tst-tls7mod-dep.
	* nptl/tst-tls7.c: New file.
	* nptl/tst-tls7mod-dep.c: New file.
	* nptl/tst-tls7mod.c: New file.

Comments

Florian Weimer Nov. 24, 2016, 11:40 a.m. | #1
On 11/24/2016 12:18 PM, Szabolcs Nagy wrote:
> Test concurrent dlopen and pthread_create when the loaded

> modules have TLS.  This triggers dl-tls assertion failures

> more reliably than the tst-stack4 test.

>

> The dlopened module has 100 DT_NEEDED dependencies and

> for me about 4000 concurrent thread creations are needed

> to see failures on x86_64.

>

> 2016-11-24  Szabolcs Nagy  <szabolcs.nagy@arm.com>

>

> 	[BZ #19329]

> 	* nptl/Makefile (tests): Add tst-tls7.

> 	(modules-names): Add tst-tls7mod, tst-tls7mod-dep.

> 	* nptl/tst-tls7.c: New file.

> 	* nptl/tst-tls7mod-dep.c: New file.

> 	* nptl/tst-tls7mod.c: New file.

>


Please do not use assert in tests.  Use the xpthread_* functions in 
test-skeleton.c instead.

This test will fail on large SMP machines due to timeouts because thread 
creation is rather expensive there.  A less resource-intense way of 
writing this test would be to iterate multiple times and try to hit the 
race each time, with a smaller number of threads created in parallel.

Florian
Szabolcs Nagy Nov. 24, 2016, 12:31 p.m. | #2
On 24/11/16 11:40, Florian Weimer wrote:
> On 11/24/2016 12:18 PM, Szabolcs Nagy wrote:

>> Test concurrent dlopen and pthread_create when the loaded

>> modules have TLS.  This triggers dl-tls assertion failures

>> more reliably than the tst-stack4 test.

>>

>> The dlopened module has 100 DT_NEEDED dependencies and

>> for me about 4000 concurrent thread creations are needed

>> to see failures on x86_64.

>>

>> 2016-11-24  Szabolcs Nagy  <szabolcs.nagy@arm.com>

>>

>>     [BZ #19329]

>>     * nptl/Makefile (tests): Add tst-tls7.

>>     (modules-names): Add tst-tls7mod, tst-tls7mod-dep.

>>     * nptl/tst-tls7.c: New file.

>>     * nptl/tst-tls7mod-dep.c: New file.

>>     * nptl/tst-tls7mod.c: New file.

>>

> 

> Please do not use assert in tests.  Use the xpthread_* functions in test-skeleton.c instead.

> 


i can do that but it looks ugly.

we are looking for an assertion failure anyway
so it would be better to make the test system
deal with that..

> This test will fail on large SMP machines due to timeouts because thread creation is rather expensive there.  A

> less resource-intense way of writing this test would be to iterate multiple times and try to hit the race each

> time, with a smaller number of threads created in parallel.

> 


i can try, but iterating is not easy: there
is dynamic linker internal state that cannot
be reset (generation counter, max dtv idx,
allocated slotininfo entries) so i'd need to
start a new process for every iteration.
Florian Weimer Nov. 24, 2016, 12:34 p.m. | #3
On 11/24/2016 01:31 PM, Szabolcs Nagy wrote:

>> Please do not use assert in tests.  Use the xpthread_* functions in test-skeleton.c instead.

>>

>

> i can do that but it looks ugly.


Well.  The asserts are incorrect because you rely on their side effects.

>> This test will fail on large SMP machines due to timeouts because thread creation is rather expensive there.  A

>> less resource-intense way of writing this test would be to iterate multiple times and try to hit the race each

>> time, with a smaller number of threads created in parallel.

>>

>

> i can try, but iterating is not easy: there

> is dynamic linker internal state that cannot

> be reset (generation counter, max dtv idx,

> allocated slotininfo entries) so i'd need to

> start a new process for every iteration.


Can you use fork before each round?

Thanks,
Florian
Szabolcs Nagy Nov. 24, 2016, 12:44 p.m. | #4
On 24/11/16 12:34, Florian Weimer wrote:
> On 11/24/2016 01:31 PM, Szabolcs Nagy wrote:

> 

>>> Please do not use assert in tests.  Use the xpthread_* functions in test-skeleton.c instead.

>>>

>>

>> i can do that but it looks ugly.

> 

> Well.  The asserts are incorrect because you rely on their side effects.

> 


ah i thought you are worried that the error output
does not end up in the test .out

i can undef NDEBUG before assert.h

>>> This test will fail on large SMP machines due to timeouts because thread creation is rather expensive there.  A

>>> less resource-intense way of writing this test would be to iterate multiple times and try to hit the race each

>>> time, with a smaller number of threads created in parallel.

>>>

>>

>> i can try, but iterating is not easy: there

>> is dynamic linker internal state that cannot

>> be reset (generation counter, max dtv idx,

>> allocated slotininfo entries) so i'd need to

>> start a new process for every iteration.

> 

> Can you use fork before each round?

> 


ok, i'll try that.
Joseph Myers Nov. 24, 2016, 1:41 p.m. | #5
On Thu, 24 Nov 2016, Szabolcs Nagy wrote:

> Test concurrent dlopen and pthread_create when the loaded

> modules have TLS.  This triggers dl-tls assertion failures

> more reliably than the tst-stack4 test.

> 

> The dlopened module has 100 DT_NEEDED dependencies and

> for me about 4000 concurrent thread creations are needed

> to see failures on x86_64.


I'd be concerned about 4000 threads exceeding task or memory limits, 
possibly interfering with other things the same user is doing that also 
try to create tasks.

(I rountinely see tst-eintr1 failing with "pthread_create failed: Resource 
temporarily unavailable", sometimes terminating the test run because make 
fails to fork to run evaluate-test.sh, apparently not all threads having 
properly terminated by the time the test does.  I don't know how many 
threads that is creating.)

-- 
Joseph S. Myers
joseph@codesourcery.com
Florian Weimer Nov. 24, 2016, 6:53 p.m. | #6
On 11/24/2016 02:41 PM, Joseph Myers wrote:
> On Thu, 24 Nov 2016, Szabolcs Nagy wrote:

>

>> Test concurrent dlopen and pthread_create when the loaded

>> modules have TLS.  This triggers dl-tls assertion failures

>> more reliably than the tst-stack4 test.

>>

>> The dlopened module has 100 DT_NEEDED dependencies and

>> for me about 4000 concurrent thread creations are needed

>> to see failures on x86_64.

>

> I'd be concerned about 4000 threads exceeding task or memory limits,

> possibly interfering with other things the same user is doing that also

> try to create tasks.


Yes, reducing the number of threads which are created in parallel is 
strongly recommended.

> (I rountinely see tst-eintr1 failing with "pthread_create failed: Resource

> temporarily unavailable", sometimes terminating the test run because make

> fails to fork to run evaluate-test.sh, apparently not all threads having

> properly terminated by the time the test does.  I don't know how many

> threads that is creating.)


As far as I can tell, tst-eintr1 creates only twenty threads at most: 
ten outer threads, and each outer thread creates one inner thread.

It's likely you are running into a kernel bug, where task exit is 
reported before the kernel has deallocated the resources used by the task:

   https://bugzilla.kernel.org/show_bug.cgi?id=154011

Florian

Patch

diff --git a/nptl/Makefile b/nptl/Makefile
index 11588fe..90152d2 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -327,7 +327,7 @@  tests += tst-cancelx2 tst-cancelx3 tst-cancelx4 tst-cancelx5 \
 	 tst-oncex3 tst-oncex4
 ifeq ($(build-shared),yes)
 tests += tst-atfork2 tst-tls3 tst-tls3-malloc tst-tls4 tst-tls5 tst-_res1 \
-	 tst-fini1 tst-stackguard1
+	 tst-fini1 tst-stackguard1 tst-tls7
 tests-nolibpthread += tst-fini1
 ifeq ($(have-z-execstack),yes)
 tests += tst-execstack
@@ -338,7 +338,7 @@  modules-names = tst-atfork2mod tst-tls3mod tst-tls4moda tst-tls4modb \
 		tst-tls5mod tst-tls5moda tst-tls5modb tst-tls5modc \
 		tst-tls5modd tst-tls5mode tst-tls5modf tst-stack4mod \
 		tst-_res1mod1 tst-_res1mod2 tst-execstack-mod tst-fini1mod \
-		tst-join7mod
+		tst-join7mod tst-tls7mod tst-tls7mod-dep
 extra-test-objs += $(addsuffix .os,$(strip $(modules-names))) \
 		   tst-cleanup4aux.o tst-cleanupx4aux.o
 test-extras += $(modules-names) tst-cleanup4aux tst-cleanupx4aux
@@ -545,6 +545,18 @@  $(objpfx)tst-tls5: $(objpfx)tst-tls5mod.so $(shared-thread-library)
 LDFLAGS-tst-tls5 = $(no-as-needed)
 LDFLAGS-tst-tls5mod.so = -Wl,-soname,tst-tls5mod.so
 
+$(objpfx)tst-tls7: $(libdl) $(shared-thread-library)
+tst-tls7mod-deps = $(shell for i in 0 1 2 3 4 5 6 7 8 9; do \
+			    for j in 0 1 2 3 4 5 6 7 8 9; do \
+			      echo $(objpfx)tst-tls7mod-dep-$$i-$$j.so; \
+			    done; done)
+$(objpfx)tst-tls7.out: $(objpfx)tst-tls7mod.so
+$(objpfx)tst-tls7mod.so: $(tst-tls7mod-deps)
+$(tst-tls7mod-deps): $(objpfx)tst-tls7mod-dep.so
+	cp -f $< $@
+clean:
+	rm -f $(tst-tls7mod-deps)
+
 ifeq ($(build-shared),yes)
 $(objpfx)tst-tls6.out: tst-tls6.sh $(objpfx)tst-tls5 \
 		       $(objpfx)tst-tls5moda.so $(objpfx)tst-tls5modb.so \
diff --git a/nptl/tst-tls7.c b/nptl/tst-tls7.c
new file mode 100644
index 0000000..e4c7db2
--- /dev/null
+++ b/nptl/tst-tls7.c
@@ -0,0 +1,62 @@ 
+/* Test concurrent dlopen and pthread_create: BZ 19329.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <pthread.h>
+#include <limits.h>
+
+#define THREADS 4000
+
+static void *
+start (void *a)
+{
+	assert (dlopen ("tst-tls7mod.so", RTLD_LAZY));
+	return 0;
+}
+
+static void *
+nop (void *a)
+{
+	return 0;
+}
+
+static int
+do_test (void)
+{
+	pthread_t td;
+	pthread_attr_t attr;
+
+	assert (pthread_attr_init (&attr) == 0);
+	assert (pthread_attr_setstacksize (&attr, PTHREAD_STACK_MIN) == 0);
+
+	/* Load a module with lots of dependencies and TLS.  */
+	assert (pthread_create (&td, 0, start, 0) == 0);
+
+	/* Concurrently create lots of threads.  */
+	for (int i = 0; i < THREADS; i++)
+		assert (pthread_create (&(pthread_t){0}, &attr, nop, 0) == 0);
+
+	/* Wait for the loading to finish, ignore other threads.  */
+	assert (pthread_join (td, 0) == 0);
+
+	return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/nptl/tst-tls7mod-dep.c b/nptl/tst-tls7mod-dep.c
new file mode 100644
index 0000000..206ece4
--- /dev/null
+++ b/nptl/tst-tls7mod-dep.c
@@ -0,0 +1 @@ 
+int __thread x;
diff --git a/nptl/tst-tls7mod.c b/nptl/tst-tls7mod.c
new file mode 100644
index 0000000..206ece4
--- /dev/null
+++ b/nptl/tst-tls7mod.c
@@ -0,0 +1 @@ 
+int __thread x;