[pulseaudio-discuss] [PATCH] Plug some memory leaks and an invalid read

[Colin Guthrie]

'Twas brillig, and David Henningsson at 16/08/11 06:14 did gyre and gimble:
> On 08/15/2011 08:52 PM, Maarten Bosmans wrote:
>> 2011/8/12 Maarten Bosmans<mkbosmans at gmail.com>:
>>> There's still another invalid read reported by valgrind.
> 
> First - thanks for having a look at all these valgrind issues!
> 
>>> The location
>>> is the same as it was for the protocol-dbus.c error of the patch: the
>>> hasmap_entry->key points to already freed memory.
>>>
>>>   Invalid read of size 1
>>>     at 0x41070CE: pa_idxset_string_hash_func (idxset.c:69)
>>>     by 0x4106064: remove_entry (hashmap.c:94)
>>>     by 0x41061FD: pa_hashmap_free (hashmap.c:111)
>>>     by 0x69C4626: pa_dbusiface_card_free (iface-card.c:548)
>>>     by 0x69D1CEA: free_card_cb (iface-core.c:2054)
>>>     by 0x4106215: pa_hashmap_free (hashmap.c:114)
>>>     by 0x69D2105: pa_dbusiface_core_free (iface-core.c:2103)
>>>     by 0x69E7169: module_dbus_protocol_LTX_pa__done
>>> (module-dbus-protocol.c:595)
>>>     by 0x406DC51: pa_module_free (module.c:162)
>>>     by 0x406E01D: pa_module_unload_all (module.c:210)
>>>     by 0x4068842: core_free (core.c:169)
>>>     by 0x406FD5D: pa_object_unref (object.c:64)
>>>   Address 0x581edd8 is 0 bytes inside a block of size 21 free'd
>>>     at 0x4025BF0: free (vg_replace_malloc.c:366)
>>>     by 0x40F128C: pa_xfree (xmalloc.c:131)
>>>     by 0x4063F4B: pa_card_profile_free (card.c:59)
>>>     by 0x40649D7: pa_card_free (card.c:202)
>>>     by 0x5051C75: module_alsa_card_LTX_pa__done (module-alsa-card.c:466)
>>>     by 0x406DC51: pa_module_free (module.c:162)
>>>     by 0x406E01D: pa_module_unload_all (module.c:210)
>>>     by 0x4068842: core_free (core.c:169)
>>>     by 0x406FD5D: pa_object_unref (object.c:64)
>>>     by 0x805224D: pa_core_unref (core.h:184)
>>>     by 0x805560B: main (main.c:1159)
>>>
>>> This one is more difficult to solve though, because the card.profile
>>> is used in the hashmaps of two modules (alsa-card and dbus-protocol)
>>
>> I dug a bit further into this.
>>
>> pa_dbusiface_card_new loops over all the pa_card_profiles of a card
>> and calls pa_dbusiface_card_profile_new to add a
>> pa_dbusiface_card_profile to its own profiles hashmap.
>> (modules/dbus/iface-card.c 524)
>> The pa_dbusiface_card_profile stores a pointer to the pa_card_profile
>> and uses its ->name as hasmap key.
>> Upon unloading the module-dbus-protocol, the profiles hashmap is
>> freed, but in the process the hashmap key is read while that string
>> (profile->profile->name) is already freed by the module that initially
>> constructed the pa_card_profile (module-alsa-card), triggering the
>> invalid read.
>>
>> So one solution would be to make profiles refcounted and another to
>> make sure module-dbus-protocol is unloaded first, but I have no idea
>> how to do that.
> 
> Please do not make profiles refcounted at this point - my jack detection
> patches already make ports refcounted (although that can be disputed)
> and we might risk circular links, which refcounting cannot handle
> properly. Given the discussion Colin, Arun and I had last week I might
> end up making profiles (instead of ports) refcounted, but I have to
> think that through first (and the jack detection might not be in v1.0
> anyway).
> 
> Instead, I think there should be a card_unlink (or similar) hook that
> module-dbus-protocol could use to remove the card and its profiles from
> the hashmap in module-dbus-protocol. Cards can disappear during runtime
> (e g when unplugging a USB headset), so this seems to be a problem not
> only at shutdown.

Just out of curosity, is this what is causing:

I: [pulseaudio] module.c: Unloading "module-alsa-card" (index: #4).
D: [pulseaudio] protocol-dbus.c: Interface org.PulseAudio.Core1.Device
removed from object /org/pulseaudio/core1/sink0
D: [pulseaudio] protocol-dbus.c: Interface org.PulseAudio.Core1.Sink
removed from object /org/pulseaudio/core1/sink0
E: [pulseaudio] core.h: Assertion 'pa_object_refcnt(pa_object_cast(o)) >
0' failed at ./pulsecore/core.h:184, function pa_core_assert_ref().
Aborting.
Aborted


?

Col



-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mageia Contributor [http://www.mageia.org/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]