On 10.04.2018 13:04, Raman Shishniou wrote:
> Hello,
>
> On 04/10/2018 01:38 PM, Georg Chini wrote:
>> On 10.04.2018 10:21, Raman Shishniou wrote:
>>> Hello,
>>>
>>> On 04/09/2018 07:15 PM, 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.
>>>> ---
>>>> src/modules/bluetooth/module-bluez5-device.c | 12 +++++++++---
>>>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
>>>> index 95d288ef..b81c233c 100644
>>>> --- a/src/modules/bluetooth/module-bluez5-device.c
>>>> +++ b/src/modules/bluetooth/module-bluez5-device.c
>>>> @@ -282,9 +282,13 @@ static int sco_process_render(struct userdata *u) {
>>>> if (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_memblock_unref(memchunk.memblock);
>>>> +
>>>> + if (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;
>>>>
>>> 1. errno value can be changed during pa_memblock_unref()
>> I don't think so. The only possible system calls used during unref should be
>> calls to free() and because we always use pa_xfree(), the errno value will
>> be preserved.
>>
> I seen callback processing during memblock_free()
You are right, b->per_type.user.free_cb() is a user defined function
which could do anything. I'll send an updated version.
>
>>> 2. I think the same changes are required for a2dp_process_render() too.
>> No, a2dp_process_render() works slightly different. It keeps the memchunk
>> in userdata and tries to re-send the same block again.
>>
>>>
>>>> pa_log_error("Failed to write data to SCO socket: %s", pa_cstrerror(errno));
>>>> return -1;
>>>> @@ -296,6 +300,8 @@ static int sco_process_render(struct userdata *u) {
>>>> pa_log_error("Wrote memory block to socket only partially! %llu written, wanted to write %llu.",
>>>> (unsigned long long) l,
>>>> (unsigned long long) memchunk.length);
>>>> +
>>>> + pa_memblock_unref(memchunk.memblock);
>>>> return -1;
>>>> }
>>>>
>>> --
>>> Raman
>>
