From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS22989 209.51.188.0/24 X-Spam-Status: No, score=-4.0 required=3.0 tests=AWL,BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id C710C1F45E for ; Sun, 16 Feb 2020 12:53:02 +0000 (UTC) Received: from localhost ([::1]:60438 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1j3JPx-0005fF-HO for normalperson@yhbt.net; Sun, 16 Feb 2020 07:53:01 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:53907) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1j3JPs-0005Ur-FM for bug-gnulib@gnu.org; Sun, 16 Feb 2020 07:52:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1j3JPq-0002li-SX for bug-gnulib@gnu.org; Sun, 16 Feb 2020 07:52:56 -0500 Received: from mo6-p00-ob.smtp.rzone.de ([2a01:238:20a:202:5300::3]:13244) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1j3JPq-0002kT-3V for bug-gnulib@gnu.org; Sun, 16 Feb 2020 07:52:54 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1581857569; s=strato-dkim-0002; d=clisp.org; h=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From: X-RZG-CLASS-ID:X-RZG-AUTH:From:Subject:Sender; bh=ylUYIA8DES/hyaDUMJqqwXwmpq7+myeH9ZAlPm1/x0A=; b=Xxk93cubwSUwGEHKXxCRIxBL0Mkv3Ynr8N1BRyJkCGCoWEBXFkw9fEyxxQad0wnIBy picUfllYqTeyN6vaR5P7aITRNmPnHk7VHeRdkGORLiuNBICzrNwy/hPD0jtkvJJ665bH 9AWKF/hnYnDuBO9e2Xz30BwuXanK62wjRg1UAA29VwxfvexWqCCoAMhgkIiYvHHag4JO A7DlSLUCbG5q8EQ4iX+g1GCn+1DZrvZYgpMPrK7G6U6rvmMT+1tsb10Opgvf2+Qno85j ohvRkrD8/2LZZ5WBJgN6Q/bpXzHoWs6U7ajZ0QFMJrE+TykV8W0V6FCkMFFLTEzUl5Un NrzQ== X-RZG-AUTH: ":Ln4Re0+Ic/6oZXR1YgKryK8brlshOcZlIWs+iCP5vnk6shH+AHjwLuWOH6fzxfs=" X-RZG-CLASS-ID: mo00 Received: from bruno.haible.de by smtp.strato.de (RZmta 46.1.12 DYNA|AUTH) with ESMTPSA id g00701w1GCqnCzD (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (curve X9_62_prime256v1 with 256 ECDH bits, eq. 3072 bits RSA)) (Client did not present a certificate); Sun, 16 Feb 2020 13:52:49 +0100 (CET) From: Bruno Haible To: Akim Demaille Subject: Re: fstrcmp: memory is not reclaimed on exit Date: Sun, 16 Feb 2020 13:52:48 +0100 Message-ID: <5090405.yQzE4uMHgr@omega> User-Agent: KMail/5.1.3 (Linux/4.4.0-171-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: <85BF43E8-5844-48B2-B7B9-F9BCA7F67E7A@lrde.epita.fr> References: <2875969.BTBdedgLeP@omega> <85BF43E8-5844-48B2-B7B9-F9BCA7F67E7A@lrde.epita.fr> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a01:238:20a:202:5300::3 X-BeenThere: bug-gnulib@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Gnulib discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: bug-gnulib@gnu.org Errors-To: bug-gnulib-bounces+normalperson=yhbt.net@gnu.org Sender: "bug-gnulib" Hi Akim, Sorry for the delay. > > Do the threads still exist at the moment valgrind does its inventory of left- > > over memory? > > I don't know when it runs its stuff, but I expect, given where it stands, > that it does it as the latest possible instant. > > > In particular: > > - Did you create threads, in which fstrcmp is run? If yes, are they still > > running? > > No, Bison does not use any threads. OK, then valgrind is really complaining about the memory allocations done in the main thread. > > - Or did you run fstrcmp in the main thread? Most likely valgrind does its > > inventory in the main thread, during exit(). This means that at this point > > the fstrcmp buffer for the main thread still exists. In other words, you > > should treat thread-local memory allocations for the main thread like > > static memory allocations (e.g. like in uniqstr.c). > > I agree, I would like to be able to explicitly release the memory. But > I can't see any API to do that in fstrcmp.c. Is this one ok? The GNU Coding Standards [1] tell us to not worry about this situation. However, the alternative - to check for a memory leak - would be to run the test in a loop (e.g. 1000 times), which is not practical since bison tests take some noticeable time to execute already when executed once. Therefore, I'm OK to look at a workaround. > +void > +fstrcmp_free (void) > +{ > + gl_once (keys_init_once, keys_init); > + gl_tls_key_destroy (buffer_key); > + gl_tls_key_destroy (bufmax_key); > +} This workaround is insufficient, since POSIX [2] says that "It is the responsibility of the application to free any application storage or perform any cleanup actions for data structures related to the deleted key or associated thread-specific data in any threads" In other words, pthread_key_delete is not guaranteed to call the destructor of 'buffer_key'. The gnulib test (tests/test-tls.c functions test_tls_dtorcheck1 and test_tls_dtorcheck2) verifies that the destructor gets called, but only for threads != main thread, and you are interested in the main thread particularly. Most likely, in this test, the destructor gets called when the thread exits [3], not when pthread_key_delete gets called. [1] https://www.gnu.org/prep/standards/html_node/Memory-Usage.html [2] https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_key_delete.html [3] https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_key_create.html This patch, however, should work. 2020-02-16 Bruno Haible fstrcmp: Add API to clean up resources. Reported by Akim Demaille in . * lib/fstrcmp.h (fstrcmp_free_resources): New declaration. * lib/fstrcmp.c (fstrcmp_free_resources): New function. diff --git a/lib/fstrcmp.c b/lib/fstrcmp.c index c6a6638..1a4fbfd 100644 --- a/lib/fstrcmp.c +++ b/lib/fstrcmp.c @@ -73,6 +73,21 @@ keys_init (void) /* Ensure that keys_init is called once only. */ gl_once_define(static, keys_init_once) +void +fstrcmp_free_resources (void) +{ + ptrdiff_t *buffer; + + gl_once (keys_init_once, keys_init); + buffer = gl_tls_get (buffer_key); + if (buffer != NULL) + { + gl_tls_set (buffer_key, NULL); + gl_tls_set (bufmax_key, (void *) (uintptr_t) 0); + free (buffer); + } +} + /* In the code below, branch probabilities were measured by Ralf Wildenhues, by running "msgmerge LL.po coreutils.pot" with msgmerge 0.18 for many diff --git a/lib/fstrcmp.h b/lib/fstrcmp.h index 92b67e3..37df588 100644 --- a/lib/fstrcmp.h +++ b/lib/fstrcmp.h @@ -38,6 +38,15 @@ extern double fstrcmp_bounded (const char *s1, const char *s2, /* A shortcut for fstrcmp. Avoids a function call. */ #define fstrcmp(s1,s2) fstrcmp_bounded (s1, s2, 0.0) +/* Frees the per-thread resources allocated by this module for the current + thread. + You don't need to call this function in threads other than the main thread, + because per-thread resources are reclaimed automatically when the thread + exits. However, per-thread resources allocated by the main thread are + comparable to static allocations; calling this function can be useful to + avoid an error report from valgrind. */ +extern void fstrcmp_free_resources (void); + #ifdef __cplusplus } #endif