Return-Path: Date: Tue, 10 Jun 2014 10:57:10 +0300 From: Andrei Emeltchenko To: Luiz Augusto von Dentz Cc: "linux-bluetooth@vger.kernel.org" Subject: Re: [RFC] android/hal-audio: Fix leaving open socket Message-ID: <20140610075709.GA16337@aemeltch-MOBL1> References: <1401971470-29048-1-git-send-email-Andrei.Emeltchenko.news@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Luiz, On Mon, Jun 09, 2014 at 04:31:13PM +0300, Luiz Augusto von Dentz wrote: > Hi Andrei, > > On Thu, Jun 5, 2014 at 3:31 PM, Andrei Emeltchenko > wrote: > > From: Andrei Emeltchenko > > > > When getting out of the poll loop we shall close socket always. > > --- > > android/hal-audio.c | 12 +++++------- > > 1 file changed, 5 insertions(+), 7 deletions(-) > > > > diff --git a/android/hal-audio.c b/android/hal-audio.c > > index bfc102f..439b583 100644 > > --- a/android/hal-audio.c > > +++ b/android/hal-audio.c > > @@ -1346,14 +1346,12 @@ static void *ipc_handler(void *data) > > /* Check if socket is still alive. Empty while loop.*/ > > while (poll(&pfd, 1, -1) < 0 && errno == EINTR); > > > > - if (pfd.revents & (POLLHUP | POLLERR | POLLNVAL)) { > > - info("Audio HAL: Socket closed"); > > + info("Audio HAL: Socket closed"); > > > > - pthread_mutex_lock(&sk_mutex); > > - close(audio_sk); > > - audio_sk = -1; > > - pthread_mutex_unlock(&sk_mutex); > > - } > > + pthread_mutex_lock(&sk_mutex); > > + close(audio_sk); > > + audio_sk = -1; > > + pthread_mutex_unlock(&sk_mutex); > > } > > > > /* audio_sk is closed at this point, just cleanup endpoints states */ > > -- > > 1.8.3.2 > > Im not quite sure what this does fix, I mean I do understand that the > if statement might not be required after all we have since we have > pfd.events = POLLHUP | POLLERR | POLLNVAL but removing it is not a fix > more a cleanup. Yes, it might be more cleanup. But the fact you check for EINTR implies that there are other errors unchecked in which case we have resource leak. Best regards Andrei Emeltchenko