From patchwork Thu Sep 21 15:04:14 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Pluzhnikov X-Patchwork-Id: 23059 Received: (qmail 93060 invoked by alias); 21 Sep 2017 15:04:50 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 92770 invoked by uid 89); 21 Sep 2017 15:04:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-vk0-f47.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=eNJfl4Ie9apEps6/sgx5DFqSEro6/erPx1m1g6EEHQY=; b=e+a+sco0JWtIpuBFxV+2Wab93EmcXM8/u/6VN7zdta3sA213l2wYh4GIarUJQ+U+x0 57G1dhcz4XXK9lhPtRQYR14VcRRBgN4oTkBTW5Iz1YLPpo1UNk4C0ZthXh1hJU+ZhK2n 905uSgqL5nl1HWfNqN5WqjB09Yuas9jFokdGTDNjA9Au1Jo+OFnjkyE5cBkc1dthInaK J88uf6y4vguhm24M1RmhvQKt/wh1QQHTpQ1Ix19EcEXE2jCB0ZiCCKi4/ZdB0Rftru20 vgZic38a6m3HPQZ5bNZa8/mvT75f6/c/ZesGHqxpjwfibL5ftaHmIwVaPYTbL3C+m8W1 vI8A== X-Gm-Message-State: AHPjjUhFMcunPvlOVyi9aW6ti7VBGS5rsDydOftA1v0V6cGmxqOv8E0T UtKmA5SrFewLZ7ZvxWQdGyfJogKZgY0RKiY/YvKiG0Re X-Google-Smtp-Source: AOwi7QAbTwgKCfGDQs4hQHf+LivKcrN6jxPeD1uc251eHnwCNbiSvIiuK4WhbSxktMsnX/99wVMBK0jy4sVs62ciDJs= X-Received: by 10.31.172.146 with SMTP id v140mr1907213vke.143.1506006285132; Thu, 21 Sep 2017 08:04:45 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <18577045-6757-fa79-7790-9bdae775bfd1@redhat.com> References: <18577045-6757-fa79-7790-9bdae775bfd1@redhat.com> From: Paul Pluzhnikov Date: Thu, 21 Sep 2017 08:04:14 -0700 Message-ID: Subject: Re: [patch] Fix dlclose / exit running in parallel resulting in dtor being called twice To: "Carlos O'Donell" Cc: GLIBC Devel On Wed, Sep 20, 2017 at 2:50 PM, Carlos O'Donell wrote: > Repost and I'll ack it. Updated patch attached. Thanks! Reviewed-by: Carlos O'Donell diff --git a/stdlib/Makefile b/stdlib/Makefile index 2fb08342e0..0a51b7bc90 100644 --- a/stdlib/Makefile +++ b/stdlib/Makefile @@ -83,7 +83,7 @@ tests := tst-strtol tst-strtod testmb testrand testsort testdiv \ tst-getrandom tst-atexit tst-at_quick_exit \ tst-cxa_atexit tst-on_exit test-atexit-race \ test-at_quick_exit-race test-cxa_atexit-race \ - test-on_exit-race + test-on_exit-race test-dlclose-exit-race tests-internal := tst-strtod1i tst-strtod3 tst-strtod4 tst-strtod5i \ tst-tls-atexit tst-tls-atexit-nodelete @@ -98,6 +98,10 @@ LDLIBS-test-at_quick_exit-race = $(shared-thread-library) LDLIBS-test-cxa_atexit-race = $(shared-thread-library) LDLIBS-test-on_exit-race = $(shared-thread-library) +LDLIBS-test-dlclose-exit-race = $(shared-thread-library) $(libdl) +LDFLAGS-test-dlclose-exit-race = $(LDFLAGS-rdynamic) +LDLIBS-test-dlclose-exit-race-helper.so = $(libsupport) $(shared-thread-library) + ifeq ($(have-cxx-thread_local),yes) CFLAGS-tst-quick_exit.o = -std=c++11 LDLIBS-tst-quick_exit = -lstdc++ @@ -108,7 +112,7 @@ else tests-unsupported += tst-quick_exit tst-thread-quick_exit endif -modules-names = tst-tls-atexit-lib +modules-names = tst-tls-atexit-lib test-dlclose-exit-race-helper extra-test-objs += $(addsuffix .os, $(modules-names)) ifeq ($(build-shared),yes) @@ -177,6 +181,7 @@ $(objpfx)tst-strtod-nan-locale.out: $(gen-locales) $(objpfx)tst-strfmon_l.out: $(gen-locales) $(objpfx)tst-strfrom.out: $(gen-locales) $(objpfx)tst-strfrom-locale.out: $(gen-locales) +$(objpfx)test-dlclose-exit-race.out: $(objpfx)test-dlclose-exit-race-helper.so endif # Testdir has to be named stdlib and needs to be writable @@ -215,6 +220,7 @@ $(objpfx)tst-strtod6: $(libm) $(objpfx)tst-strtod-nan-locale: $(libm) tst-tls-atexit-lib.so-no-z-defs = yes +test-dlclose-exit-race-helper.so-no-z-defs = yes $(objpfx)tst-tls-atexit: $(shared-thread-library) $(libdl) $(objpfx)tst-tls-atexit.out: $(objpfx)tst-tls-atexit-lib.so diff --git a/stdlib/exit.c b/stdlib/exit.c index b74f1825f0..6a354fd0af 100644 --- a/stdlib/exit.c +++ b/stdlib/exit.c @@ -69,8 +69,7 @@ __run_exit_handlers (int status, struct exit_function_list **listp, while (cur->idx > 0) { - const struct exit_function *const f = - &cur->fns[--cur->idx]; + struct exit_function *const f = &cur->fns[--cur->idx]; const uint64_t new_exitfn_called = __new_exitfn_called; /* Unlock the list while we call a foreign function. */ @@ -99,6 +98,9 @@ __run_exit_handlers (int status, struct exit_function_list **listp, atfct (); break; case ef_cxa: + /* To avoid dlopen/exit race calling cxafct twice, we must + mark this function as ef_free. */ + f->flavor = ef_free; cxafct = f->func.cxa.fn; #ifdef PTR_DEMANGLE PTR_DEMANGLE (cxafct); diff --git a/stdlib/test-dlclose-exit-race-helper.c b/stdlib/test-dlclose-exit-race-helper.c new file mode 100644 index 0000000000..1b443e0b52 --- /dev/null +++ b/stdlib/test-dlclose-exit-race-helper.c @@ -0,0 +1,80 @@ +/* Helper for exit/dlclose race test. + Copyright (C) 2017 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 + . */ + +#include +#include +#include +#include +#include +#include + +/* Semaphore defined in executable to ensure we have a happens-before + between the first function starting and exit being called. */ +extern sem_t order1; + +/* Semaphore defined in executable to ensure we have a happens-before + between the second function starting and the first function returning. */ +extern sem_t order2; + +/* glibc function for registering DSO-specific exit functions. */ +extern int __cxa_atexit (void (*func) (void *), void *arg, void *dso_handle); + +/* Hidden compiler handle to this shared object. */ +extern void *__dso_handle __attribute__ ((__weak__)); + +static void +first (void *start) +{ + /* Let the exiting thread run. */ + sem_post (&order1); + + /* Wait for exiting thread to finish. */ + sem_wait (&order2); + + printf ("first\n"); +} + +static void +second (void *start) +{ + /* We may be called from different threads. + This lock protects called. */ + static pthread_mutex_t mtx = PTHREAD_MUTEX_INITIALIZER; + static bool called = false; + + xpthread_mutex_lock (&mtx); + if (called) + { + fprintf (stderr, "second called twice!\n"); + abort (); + } + called = true; + xpthread_mutex_unlock (&mtx); + + printf ("second\n"); +} + + +__attribute__ ((constructor)) static void +constructor (void) +{ + sem_init (&order1, 0, 0); + sem_init (&order2, 0, 0); + __cxa_atexit (second, NULL, __dso_handle); + __cxa_atexit (first, NULL, __dso_handle); +} diff --git a/stdlib/test-dlclose-exit-race.c b/stdlib/test-dlclose-exit-race.c new file mode 100644 index 0000000000..d822c8bae9 --- /dev/null +++ b/stdlib/test-dlclose-exit-race.c @@ -0,0 +1,80 @@ +/* Test for exit/dlclose race. + Copyright (C) 2017 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 + . */ + +/* This file must be run from within a directory called "stdlib". */ + +/* This test verifies that when dlopen in one thread races against exit + in another thread, we don't call registered destructor twice. + + Expected result: + second + first + ... clean termination +*/ + +#include +#include +#include +#include +#include +#include + +/* Semaphore to ensure we have a happens-before between the first function + starting and exit being called. */ +sem_t order1; + +/* Semaphore to ensure we have a happens-before between the second function + starting and the first function returning. */ +sem_t order2; + +void * +exit_thread (void *arg) +{ + /* Wait for the dlclose to start... */ + sem_wait (&order1); + /* Then try to run the exit sequence which should call all + __cxa_atexit registered functions and in parallel with + the executing dlclose(). */ + exit (0); +} + + +void +last (void) +{ + /* Let dlclose thread proceed. */ + sem_post (&order2); +} + +int +main (void) +{ + void *dso; + pthread_t thread; + + atexit (last); + + dso = xdlopen ("$ORIGIN/test-dlclose-exit-race-helper.so", + RTLD_NOW|RTLD_GLOBAL); + thread = xpthread_create (NULL, exit_thread, NULL); + + xdlclose (dso); + xpthread_join (thread); + + FAIL_EXIT1 ("Did not terminate via exit(0) in exit_thread() as expected."); +}