[pulseaudio-discuss] [PATCH v2] bluez5-device: Fix memory leak in sco_process_render()

[Tanu Kaskinen]

On Tue, 2018-04-10 at 22:08 +0200, Georg Chini wrote:
> sco_process_render does not unref the memblock when it encounters an error.
> 
> This patch fixes the issue. It also changes the return value to 1 in the case
> of EAGAIN. Because the data was already rendered and cannot be re-sent, we
> have to discard the block.
> ---
> Changes in v2: Save errno before calling pa_memblock_unref()
> 
>  src/modules/bluetooth/module-bluez5-device.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
> index 95d288ef..640ff058 100644
> --- a/src/modules/bluetooth/module-bluez5-device.c
> +++ b/src/modules/bluetooth/module-bluez5-device.c
> @@ -253,6 +253,7 @@ static void connect_ports(struct userdata *u, void *new_data, pa_direction_t dir
>  static int sco_process_render(struct userdata *u) {
>      ssize_t l;
>      pa_memchunk memchunk;
> +    int saved_errno;
>  
>      pa_assert(u);
>      pa_assert(u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT ||
> @@ -279,14 +280,20 @@ static int sco_process_render(struct userdata *u) {
>          if (l > 0)
>              break;
>  
> -        if (errno == EINTR)
> +        saved_errno = errno;
> +
> +        if (saved_errno == EINTR)
>              /* Retry right away if we got interrupted */
>              continue;
> -        else if (errno == EAGAIN)
> -            /* Hmm, apparently the socket was not writable, give up for now */
> -            return 0;
>  
> -        pa_log_error("Failed to write data to SCO socket: %s", pa_cstrerror(errno));
> +        pa_memblock_unref(memchunk.memblock);
> +
> +        if (saved_errno == EAGAIN)
> +            /* Hmm, apparently the socket was not writable, give up for now.
> +             * Because the data was already rendered, let's discard the block. */
> +            return 1;

The modified EAGAIN handling prevents the log message about EAGAIN
after POLLOUT from being printed. Maybe the log message should be moved
 to a2dp/sco_process_render()?

-- 
Tanu

https://liberapay.com/tanuk
https://www.patreon.com/tanuk