Re: [pulseaudio-discuss] [PATCH] bluez5: Fix free order of adapters and devices
On Fri, 2014-11-28 at 13:43 +0100, David Henningsson wrote: > Because the adapters reference the devices hashmap on free, we mush > free the adapters hashmap first and then the devices hashmap. > > Reported-by: Alexander Patrakov > Signed-off-by: David Henningsson > --- > src/modules/bluetooth/bluez5-util.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Ack. I'd prefer first freeing the device and adapter objects, and only then the hashmaps, though, because that way the discovery object doesn't become invalid before all of the objects owned by it are freed, but if you don't feel like doing that, I'm fine with this patch too. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] bluez5: Fix free order of adapters and devices
28.11.2014 19:53, I wrote: 28.11.2014 17:43, David Henningsson wrote: Because the adapters reference the devices hashmap on free, we mush free the adapters hashmap first and then the devices hashmap. I think the fix is incomplete, or there is more than one problem. Sorry for not being positive enough. The patch indeed helps against valgrind errors if no audio is being streamed when pulseaudio gets a SIGTERM. The newly posted error is a pre-existing one - i.e. the patch fixes only one out of two or three problems. -- Alexander E. Patrakov ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] bluez5: Fix free order of adapters and devices
28.11.2014 17:43, David Henningsson wrote:
Because the adapters reference the devices hashmap on free, we mush
free the adapters hashmap first and then the devices hashmap.
I think the fix is incomplete, or there is more than one problem. If I
kill pulseaudio that receives audio from my phone (using a2dp profile),
I get this:
==5256== Invalid read of size 1
==5256==at 0x5D5FCE0: pa_idxset_string_hash_func (idxset.c:67)
==5256==by 0x5D5EBF5: remove_entry (hashmap.c:103)
==5256==by 0x5D5F2BB: pa_hashmap_remove_all (hashmap.c:229)
==5256==by 0x5D5F341: pa_hashmap_free (hashmap.c:120)
==5256==by 0x1E50443C: module_bluez5_discover_LTX_pa__done
(module-bluez5-discover.c:162)
==5256==by 0x4E60B48: pa_module_free (module.c:227)
==5256==by 0x4E61929: pa_module_unload_all (module.c:292)
==5256==by 0x406476: main (main.c:1161)
==5256== Address 0x1aa7d670 is 0 bytes inside a block of size 38 free'd
==5256==at 0x4C2A20C: free (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==5256==by 0x510395E: pa_xfree (xmalloc.c:131)
==5256==by 0x1E70BA4F: device_free (bluez5-util.c:433)
==5256==by 0x5D5F2DC: pa_hashmap_remove_all (hashmap.c:232)
==5256==by 0x5D5F341: pa_hashmap_free (hashmap.c:120)
==5256==by 0x1E70EAB5: pa_bluetooth_discovery_unref (bluez5-util.c:1667)
==5256==by 0x1E50442E: module_bluez5_discover_LTX_pa__done
(module-bluez5-discover.c:159)
==5256==by 0x4E60B48: pa_module_free (module.c:227)
==5256==by 0x4E61929: pa_module_unload_all (module.c:292)
==5256==by 0x406476: main (main.c:1161)
==5256==
==5256== Invalid read of size 1
==5256==at 0x5D5FCFE: pa_idxset_string_hash_func (idxset.c:67)
==5256==by 0x5D5EBF5: remove_entry (hashmap.c:103)
==5256==by 0x5D5F2BB: pa_hashmap_remove_all (hashmap.c:229)
==5256==by 0x5D5F341: pa_hashmap_free (hashmap.c:120)
==5256==by 0x1E50443C: module_bluez5_discover_LTX_pa__done
(module-bluez5-discover.c:162)
==5256==by 0x4E60B48: pa_module_free (module.c:227)
==5256==by 0x4E61929: pa_module_unload_all (module.c:292)
==5256==by 0x406476: main (main.c:1161)
==5256== Address 0x1aa7d671 is 1 bytes inside a block of size 38 free'd
==5256==at 0x4C2A20C: free (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==5256==by 0x510395E: pa_xfree (xmalloc.c:131)
==5256==by 0x1E70BA4F: device_free (bluez5-util.c:433)
==5256==by 0x5D5F2DC: pa_hashmap_remove_all (hashmap.c:232)
==5256==by 0x5D5F341: pa_hashmap_free (hashmap.c:120)
==5256==by 0x1E70EAB5: pa_bluetooth_discovery_unref (bluez5-util.c:1667)
==5256==by 0x1E50442E: module_bluez5_discover_LTX_pa__done
(module-bluez5-discover.c:159)
==5256==by 0x4E60B48: pa_module_free (module.c:227)
==5256==by 0x4E61929: pa_module_unload_all (module.c:292)
==5256==by 0x406476: main (main.c:1161)
==5256==
Reported-by: Alexander Patrakov
Signed-off-by: David Henningsson
---
src/modules/bluetooth/bluez5-util.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/modules/bluetooth/bluez5-util.c
b/src/modules/bluetooth/bluez5-util.c
index 6894e83..0b234ae 100644
--- a/src/modules/bluetooth/bluez5-util.c
+++ b/src/modules/bluetooth/bluez5-util.c
@@ -1660,12 +1660,12 @@ void
pa_bluetooth_discovery_unref(pa_bluetooth_discovery *y) {
pa_dbus_free_pending_list(&y->pending);
-if (y->devices)
-pa_hashmap_free(y->devices);
-
if (y->adapters)
pa_hashmap_free(y->adapters);
+if (y->devices)
+pa_hashmap_free(y->devices);
+
if (y->transports) {
pa_assert(pa_hashmap_isempty(y->transports));
pa_hashmap_free(y->transports);
--
Alexander E. Patrakov
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] bluez5: Fix free order of adapters and devices
Because the adapters reference the devices hashmap on free, we mush
free the adapters hashmap first and then the devices hashmap.
Reported-by: Alexander Patrakov
Signed-off-by: David Henningsson
---
src/modules/bluetooth/bluez5-util.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/modules/bluetooth/bluez5-util.c
b/src/modules/bluetooth/bluez5-util.c
index 6894e83..0b234ae 100644
--- a/src/modules/bluetooth/bluez5-util.c
+++ b/src/modules/bluetooth/bluez5-util.c
@@ -1660,12 +1660,12 @@ void
pa_bluetooth_discovery_unref(pa_bluetooth_discovery *y) {
pa_dbus_free_pending_list(&y->pending);
-if (y->devices)
-pa_hashmap_free(y->devices);
-
if (y->adapters)
pa_hashmap_free(y->adapters);
+if (y->devices)
+pa_hashmap_free(y->devices);
+
if (y->transports) {
pa_assert(pa_hashmap_isempty(y->transports));
pa_hashmap_free(y->transports);
--
2.1.3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
