Luiz reported today a bug revealed by Valgrind: ==16165== Invalid read of size 1 ==16165== at 0x5170C2E: pa_idxset_string_hash_func (idxset.c:67) ==16165== by 0x516FC77: remove_entry (hashmap.c:93) ==16165== by 0x516FE0B: pa_hashmap_free (hashmap.c:110) ==16165== by 0x4C77B75: device_port_free (device-port.c:60) ==16165== by 0x4C43C7F: pa_object_unref (object.c:64) ==16165== by 0x4C7785E: pa_device_port_unref (device-port.h:61) ==16165== by 0x4C77D9C: pa_device_port_hashmap_free (device-port.c:96) ==16165== by 0x4C860BE: source_free (source.c:680) ==16165== by 0x4C43C7F: pa_object_unref (object.c:64) ==16165== by 0x155AD610: pa_source_unref (source.h:237) ==16165== by 0x155B1965: pa_dbusiface_stream_free (iface-stream.c:917) ==16165== by 0x1559CE89: subscription_cb (iface-core.c:1746) ==16165== Address 0x10dc1ec0 is 0 bytes inside a block of size 12 free'd ==16165== at 0x4A077A6: free (vg_replace_malloc.c:446) ==16165== by 0x4F31E3D: pa_xfree (xmalloc.c:131) ==16165== by 0x4C376A8: pa_card_profile_free (card.c:62) ==16165== by 0x4C38788: pa_card_free (card.c:254) ==16165== by 0x159D6E6C: module_bluetooth_device_LTX_pa__done (module-bluetooth-device.c:2627) ==16165== by 0x4C41C02: pa_module_free (module.c:162) ==16165== by 0x4C41DE8: pa_module_unload (module.c:188) ==16165== by 0x159D6391: discovery_hook_cb (module-bluetooth-device.c:2439) ==16165== by 0x4C3EF55: pa_hook_fire (hook-list.c:106) ==16165== by 0x13528C21: run_callback (bluetooth-util.c:668) ==16165== by 0x1352E504: endpoint_clear_configuration (bluetooth-util.c:1859) ==16165== by 0x1352F23B: endpoint_handler (bluetooth-util.c:2077) The keys of the profiles hashmap of pa_device_port were getting freed too early. The reason was that module-dbus-protocol held a reference to a bluetooth source, and that source in turn held a reference to a port. When the associated card was freed, the port was not. When the port then eventually was freed, it was referencing profile memory that was already freed. I started by modifying pa_card_free(), and I found myself wanting a pa_hashmap_remove_all() function, so I started writing that. I wanted the free_cb of that function to match the free_cb type of pa_hashmap_free(), but pa_hashmap_free() used pa_free2_cb_t, which was stupid, so I changed that first. Then I made the same changes to idxset. Patches 1-7 implement this refactoring. Patches 8-13 then fix the actual problem by removing refcounting from ports and making sure that there won't be any other reference holders when the sink or source implementor calls pa_sink/source_unref(). This ensures that the ports and profiles are freed in the expected order when a card is removed. FWIW, three of the patches are from an older patch set that I haven't finished (the intention has been to completely fix https://bugs.freedesktop.org/show_bug.cgi?id=45817 before submitting the patch set, but I haven't worked on that lately). Tanu Kaskinen (13): gconf: Add userdata pointer to struct module_info gconf: Remove needless userdata function arguments hashmap: Use pa_free_cb_t instead of pa_free2_cb_t device-port: Remove pa_device_port_hashmap_free() idxset: Use pa_free_cb_t instead of pa_free2_cb_t hashmap: Add pa_hashmap_remove_all() idxset: Add pa_idxset_remove_all() core: Add hooks for default device changes dbus: Track the default sink and source with hooks dbus: Track streams with hooks dbus: Track stream moves with hooks bluetooth: Fix thread teardown code ordering Remove refcounting from ports, sinks and sources src/modules/alsa/alsa-mixer.c | 68 +--- src/modules/alsa/alsa-sink.c | 4 +- src/modules/alsa/alsa-ucm.c | 12 +- src/modules/alsa/module-alsa-card.c | 18 +- src/modules/bluetooth/bluetooth-util.c | 4 +- src/modules/bluetooth/module-bluetooth-device.c | 22 +- src/modules/bluetooth/module-bluetooth-discover.c | 2 +- src/modules/bluetooth/module-bluetooth-proximity.c | 10 +- src/modules/dbus/iface-card.c | 10 +- src/modules/dbus/iface-core.c | 416 +++++++++----------- src/modules/dbus/iface-device.c | 23 +- src/modules/dbus/iface-stream.c | 96 ++--- src/modules/dbus/module-dbus-protocol.c | 9 +- src/modules/gconf/module-gconf.c | 37 +- src/modules/macosx/module-bonjour-publish.c | 2 +- src/modules/module-augment-properties.c | 10 +- src/modules/module-card-restore.c | 4 +- src/modules/module-combine-sink.c | 11 +- src/modules/module-console-kit.c | 9 +- src/modules/module-device-manager.c | 10 +- src/modules/module-device-restore.c | 6 +- src/modules/module-filter-apply.c | 2 +- src/modules/module-role-cork.c | 18 +- src/modules/module-role-ducking.c | 19 +- src/modules/module-stream-restore.c | 14 +- src/modules/module-suspend-on-idle.c | 15 +- src/modules/module-systemd-login.c | 14 +- src/modules/module-udev-detect.c | 10 +- src/modules/module-zeroconf-discover.c | 2 +- src/modules/module-zeroconf-publish.c | 18 +- src/modules/raop/module-raop-discover.c | 2 +- src/modules/rtp/headerlist.c | 7 +- src/modules/rtp/module-rtp-recv.c | 17 +- src/pulse/context.c | 4 +- src/pulse/format.c | 4 - src/pulse/internal.h | 2 - src/pulse/proplist.c | 7 +- src/pulsecore/card.c | 38 +- src/pulsecore/client.c | 4 +- src/pulsecore/core-scache.c | 5 +- src/pulsecore/core.c | 20 +- src/pulsecore/core.h | 2 + src/pulsecore/database-simple.c | 7 +- src/pulsecore/device-port.c | 30 +- src/pulsecore/device-port.h | 7 +- src/pulsecore/hashmap.c | 25 +- src/pulsecore/hashmap.h | 7 +- src/pulsecore/idxset.c | 25 +- src/pulsecore/idxset.h | 10 +- src/pulsecore/memblock.c | 4 +- src/pulsecore/modargs.c | 6 +- src/pulsecore/module.c | 4 +- src/pulsecore/mutex-win32.c | 2 +- src/pulsecore/namereg.c | 2 + src/pulsecore/protocol-cli.c | 2 +- src/pulsecore/protocol-dbus.c | 66 +--- src/pulsecore/protocol-esound.c | 2 +- src/pulsecore/protocol-http.c | 2 +- src/pulsecore/protocol-native.c | 18 +- src/pulsecore/protocol-simple.c | 2 +- src/pulsecore/sink-input.c | 27 +- src/pulsecore/sink.c | 33 +- src/pulsecore/source-output.c | 10 +- src/pulsecore/source.c | 17 +- 64 files changed, 521 insertions(+), 794 deletions(-) -- 1.7.10.4
