From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 7F78B2082; Mon, 6 May 2019 15:22:13 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C8A3081DF4; Mon, 6 May 2019 13:22:12 +0000 (UTC) Received: from dhcp-25.97.bos.redhat.com (ovpn-124-136.rdu2.redhat.com [10.10.124.136]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9E7DE1EE; Mon, 6 May 2019 13:22:09 +0000 (UTC) From: Aaron Conole To: David Marchand Cc: dev , Herakliusz Lipiec , dpdk stable , Anatoly Burakov References: <20190506131120.25140-1-aconole@redhat.com> Date: Mon, 06 May 2019 09:22:08 -0400 In-Reply-To: (David Marchand's message of "Mon, 6 May 2019 15:14:37 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Mon, 06 May 2019 13:22:12 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH] ipc: unlock on failure X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 May 2019 13:22:13 -0000 David Marchand writes: > On Mon, May 6, 2019 at 3:11 PM Aaron Conole wrote: > > Reported by Coverity. > > Fixes: a2a06860b8c4 ("ipc: fix memory leak on request failure") > Cc: Herakliusz Lipiec > Cc: stable@dpdk.org > Cc: Anatoly Burakov > Signed-off-by: Aaron Conole > --- > lib/librte_eal/common/eal_common_proc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c > index d23728604..3498e6b07 100644 > --- a/lib/librte_eal/common/eal_common_proc.c > +++ b/lib/librte_eal/common/eal_common_proc.c > @@ -1005,8 +1005,10 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply, > /* unlocks the mutex while waiting for response, > * locks on receive > */ > - if (mp_request_sync(path, req, reply, &end)) > + if (mp_request_sync(path, req, reply, &end)) { > + pthread_mutex_unlock(&pending_requests.lock); > goto err; > + } > } > pthread_mutex_unlock(&pending_requests.lock); > /* unlock the directory */ > -- > 2.19.1 > > Unfortunately, I think this is more complex than this. > We will leave a flock() pending and leak some resources from opendir(). I completely forgot about it, while double checking the mp_request_sync You're right, we leak the dirfd, still. Okay, I think see a better way to write this. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 8D304A0096 for ; Mon, 6 May 2019 15:22:16 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 463582BAB; Mon, 6 May 2019 15:22:15 +0200 (CEST) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 7F78B2082; Mon, 6 May 2019 15:22:13 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C8A3081DF4; Mon, 6 May 2019 13:22:12 +0000 (UTC) Received: from dhcp-25.97.bos.redhat.com (ovpn-124-136.rdu2.redhat.com [10.10.124.136]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9E7DE1EE; Mon, 6 May 2019 13:22:09 +0000 (UTC) From: Aaron Conole To: David Marchand Cc: dev , Herakliusz Lipiec , dpdk stable , Anatoly Burakov References: <20190506131120.25140-1-aconole@redhat.com> Date: Mon, 06 May 2019 09:22:08 -0400 In-Reply-To: (David Marchand's message of "Mon, 6 May 2019 15:14:37 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Mon, 06 May 2019 13:22:12 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH] ipc: unlock on failure X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190506132208.a2YH3LK8MOBDIDT0GN8UAMlhLf1W_3vFyXt2KN_CYGA@z> David Marchand writes: > On Mon, May 6, 2019 at 3:11 PM Aaron Conole wrote: > > Reported by Coverity. > > Fixes: a2a06860b8c4 ("ipc: fix memory leak on request failure") > Cc: Herakliusz Lipiec > Cc: stable@dpdk.org > Cc: Anatoly Burakov > Signed-off-by: Aaron Conole > --- > lib/librte_eal/common/eal_common_proc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c > index d23728604..3498e6b07 100644 > --- a/lib/librte_eal/common/eal_common_proc.c > +++ b/lib/librte_eal/common/eal_common_proc.c > @@ -1005,8 +1005,10 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply, > /* unlocks the mutex while waiting for response, > * locks on receive > */ > - if (mp_request_sync(path, req, reply, &end)) > + if (mp_request_sync(path, req, reply, &end)) { > + pthread_mutex_unlock(&pending_requests.lock); > goto err; > + } > } > pthread_mutex_unlock(&pending_requests.lock); > /* unlock the directory */ > -- > 2.19.1 > > Unfortunately, I think this is more complex than this. > We will leave a flock() pending and leak some resources from opendir(). I completely forgot about it, while double checking the mp_request_sync You're right, we leak the dirfd, still. Okay, I think see a better way to write this.