From patchwork Wed Sep 20 18:30:30 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Pluzhnikov X-Patchwork-Id: 816397 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-84796-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="mXhvE3Uz"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xy7c66455z9s7g for ; Thu, 21 Sep 2017 04:31:18 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:from:date:message-id:subject:to :content-type; q=dns; s=default; b=E9Ce+nsruLtSNESdrP8SOz1rLtzXc umaNBRkYj1u6WFRD+5iUupDODCXVsJCZq/3V/C4eVyIllxOmAQ6a7odXPy+z/VGe o/CfB+qS7elZ4tpfDBZsK4QQeJ+jhcSAwblRspBAjS8j8BIhi4jq9pOyFnPEjLOd ZyN8TZG80rYDlg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:from:date:message-id:subject:to :content-type; s=default; bh=gF0ZBhW59kvQEFSvuIaRLJ1maOM=; b=mXh vE3UzKoZlSNVGPyJWkAObxp5ed/3wNucR0Spmszau7hLdYPivcla9Ocuy6IGILWp RxsT2LKOsFUId9wQgAwQRpa37i2pf883JWX4i6YKfhjmYjg5PwMRcsDnuKFBrlN4 N6t5xx7YD4XXHYtHOE1ZhOFqOYv3nVsj0Mx2/4M8= Received: (qmail 109847 invoked by alias); 20 Sep 2017 18:31:12 -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 109828 invoked by uid 89); 20 Sep 2017 18:31:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.0 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=races, ORIGIN X-HELO: mail-ua0-f172.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:from:date:message-id:subject:to; bh=UrgSggFshC3VrjmsnmeljOBEWzRn4lXB7Kyl2hcZtYc=; b=mOheyw+y5hUhpVDr5zEcXlTt7i85szB1AsffhcrU09qbWAbvEmxbPFrWXc78ENu6b3 av31r8Vm9ktaCEF6BGe3rYZHL8zt5NzA6fyo/nxXhLPg1xlVuVNIWU96lsL4Y/hgOJUG 3Bhb7H7jewQ91PuPOhAOpU+DgMaJHxtu5oVqfI0b9mrTwcytltCxSyOa3NeP9m9+1zWo ymA/EUx7r3vZ2lFLCxR2r0ZWxIkgk4kTTuY9Pzey/uPDvIchyfbCVMkL2b73UNFbq1U7 IBGWdguOjSaoThd7mqh9N5AMx80HXDsPvJry2b/kVD30UpsC2Z1FgH43lpU/KS8EjqHE YDMA== X-Gm-Message-State: AHPjjUinl6sIdeRqvFu4X8i3/spcP99Zf95MHM81b0RZ6wpqZKPXvQF1 aXbXmr1CbMBJAgz+4uMrxco8WZFCZE/+oLMkHULq08u1R7Q= X-Google-Smtp-Source: AOwi7QBA7KPvdZP99iQiB6VYl87NhUsEVxC+M6XIrkZde7e7AqJNNt+Ont44vX9/Zj6gIsPyb1kibzg0bNoIhnt8qDE= X-Received: by 10.176.24.99 with SMTP id j35mr5376445uag.155.1505932261671; Wed, 20 Sep 2017 11:31:01 -0700 (PDT) MIME-Version: 1.0 From: Paul Pluzhnikov Date: Wed, 20 Sep 2017 11:30:30 -0700 Message-ID: Subject: [patch] Fix dlclose / exit running in parallel resulting in dtor being called twice To: GLIBC Devel , "Carlos O'Donell" Greetings, This a followup to and fix for the problem discovered in https://sourceware.org/ml/libc-alpha/2017-09/msg00762.html. As the test program in above message shows, dlclose running in parallel with exit currently results in "second" being called twice. The fix for this is trivial (2 line change in stdlib/exit.c). The majority of this patch is the test case, which is modified from original to avoid sleep. Thanks, P.S. Not sure what commit message should be for this patch. Reviewed-by: Carlos O'Donell diff --git a/stdlib/Makefile b/stdlib/Makefile index 2fb08342e0..443230bbb7 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 = $(libsupport) $(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..bbb97c7c64 --- /dev/null +++ b/stdlib/test-dlclose-exit-race.c @@ -0,0 +1,116 @@ +/* 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 + +/* 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 * +open_library (const char * pathname) +{ + void *dso; + char *err; + + /* Open the DSO. */ + dso = dlopen (pathname, RTLD_NOW|RTLD_GLOBAL); + if (dso == NULL) + { + err = dlerror (); + fprintf (stderr, "%s\n", err); + exit (1); + } + /* Clear any errors. */ + dlerror (); + return dso; +} + +int +close_library (void *dso) +{ + int ret; + char *err; + + /* Close the library and look for errors too. */ + ret = dlclose (dso); + if (ret != 0) + { + err = dlerror (); + fprintf (stderr, "%s\n", err); + exit (1); + } + return ret; +} + +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 = open_library ("$ORIGIN/test-dlclose-exit-race-helper.so"); + thread = xpthread_create (NULL, exit_thread, NULL); + + close_library (dso); + xpthread_join (thread); + + /* Unreached. */ + fprintf (stderr, "Reached unreachable code."); + abort (); +}