From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 5mO0MoE/lGBMLAAAWB0awg (envelope-from ) for ; Thu, 06 May 2021 15:12:01 -0400 Received: by simark.ca (Postfix, from userid 112) id C15741F11C; Thu, 6 May 2021 15:12:01 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 29BDE1E54D for ; Thu, 6 May 2021 15:12:01 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BA9C8388F030; Thu, 6 May 2021 19:12:00 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BA9C8388F030 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1620328320; bh=kVD3lBtvTZ4j+nREhmw+U770MxRUEIZ2Rw5sV9tKhyk=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=NvXLaGKMWRyuTUR67Z2FqnqDrmtxTL8N3VB/VFuQH8gZmZ22R0AxSoCdQuOSwVtR9 37Tjknt7FFcUZnVnhmn+VgfaRZivYVs/IUqtfovwtdE4fXACvzuh5Ts5ueSKI5ctbV pXgcsW6h+3WZj8JsS98Rcz1o8qPhkxruWC2i+9Hc= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 84194388F030 for ; Thu, 6 May 2021 19:11:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 84194388F030 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 146JBpPl027053 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 6 May 2021 15:11:56 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 146JBpPl027053 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 5353E1E54D; Thu, 6 May 2021 15:11:51 -0400 (EDT) Subject: Re: [PATCH] debuginfod-support.c: Use long-lived debuginfod_client To: "Frank Ch. Eigler" References: <20210430235735.1371915-1-amerey@redhat.com> <87eeek1y07.fsf@redhat.com> <20210506184707.GA29815@redhat.com> Message-ID: Date: Thu, 6 May 2021 15:11:51 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210506184707.GA29815@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Thu, 6 May 2021 19:11:51 +0000 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Cc: Tom Tromey , gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 2021-05-06 2:47 p.m., Frank Ch. Eigler wrote:> Hi - > >> I think it would still be a good idea to properly close the client >> at exit. Not because the memory leak is a concern, but because we >> shouldn't assume what closing a debuginfod client (current or >> future) does or does not do, so we shouldn't assume that we can get >> away without calling debuginfo_end. > > If it's not hard to do, sure .... but wouldn't that necessitate > exposing the pointer from function static scope again? I don't think so, just make the static variable a unique pointer (the type that you are removing in this patch). The destructor of the unique_ptr will be called at program exit, closing the client cleanly. That variable can be declared inside debuginfod_init, like Tom suggested. Something like: static debuginfo_client * debuginfod_init () { static debuginfod_client_up global_client; if (global_client == nullptr) global_client.reset (debuginfod_begin ()); return global_client.get (); } In that case, the debuginfod_init function should probably be renamed to something like get_debuginfod_client, something like that. >> A debuginfod client could eventually maintain some temporary files >> that need to be deleted when closing in order not to litter /tmp, >> some cache files need to be flushed, etc. > > I suspect that if we were to do any of those things in the future, > we'd keep things clean by techniques like holding onto fd's to > unlinked files, so process exit is enough. (We could make a > commitment in the docs.) That sounds like a good idea, this way even if the user of the client crashes, there won't be any leftovers. However, committing to that in the documentation just sounds like an unnecessary way to paint yourself in a corner. Simon