| Summary: | pulseaudio server crashes on bluetooth disconnect | ||
|---|---|---|---|
| Product: | [Build System, Metadata & Runtime] OE-Core | Reporter: | Daniel Thompson <daniel.thompson> |
| Component: | multimedia | Assignee: | Jussi Kukkonen <jku> |
| Status: | RESOLVED FIXED | QA Contact: | David Rodriguez <david.israelx.rodriguez.castellanos> |
| Severity: | normal | ||
| Priority: | Medium+ | CC: | jose.perez.carranza, maxin.john, meta.mr.watcher, meta.watcher, ndec13, richard.purdie, tanuk |
| Version: | 2.0 | ||
| Target Milestone: | 2.2 M3 | ||
| Hardware: | All | ||
| OS: | arm64 | ||
| Whiteboard: | |||
| OS type for building Yocto: | --- | Type of Regression: | --- |
| Verified: | Documentation change: | No (bug/feature does not impact docs) | |
Oh... I forgot to mention. The line numbers in the valgrind report are based on a master build rather than a jethro build (the issue reproduces pretty much identically in both versions). Adding Tanu I expect this bug to be fixed by this patch (currently in master-next): https://patchwork.openembedded.org/patch/127557/ - or at least the stack trace will be different. Is a less drastic fix needed for jethro and krogoth than upgrading pulseaudio to 9.0? (In reply to comment #3) > I expect this bug to be fixed by this patch (currently in master-next): > https://patchwork.openembedded.org/patch/127557/ - or at least the stack > trace will be different. Issue is resolved when this patch is applied on top of master. Thanks for quick response. Problem is resolved in master with: http://git.yoctoproject.org/cgit.cgi/poky/commit/?id=7c9acf0ea47701e7ae17c7bee11929d1926e00ad If we need this in jethro/krogoth, please backport a patch to fix in the older pulseaudio. We can't really take version upgrades like this on the older releases. (In reply to comment #6) > Problem is resolved in master with: > > http://git.yoctoproject.org/cgit.cgi/poky/commit/ > ?id=7c9acf0ea47701e7ae17c7bee11929d1926e00ad > > If we need this in jethro/krogoth, please backport a patch to fix in the > older pulseaudio. We can't really take version upgrades like this on the > older releases. well, the bug is not upstream. In fact upstream works fine, and the bug comes from a patch that we add in the OE recipe for pulseaudio. So i am not sure i would agree with your comment above.. for jethro/krogoth i would be tempted to say that we should remove the offending patches from our metadata.. the thing is we have a buggy patch that we carry.. I can make a fix for jethro/krogoth that doesn't require upgrading to 9.0 and doesn't reintroduce bug 8448. I haven't investigated the crash in much detail yet, but I would expect the fix to only require changing a few lines of code. The reason why master works is that the patches for bug 8448 had some significant changes during the upstreaming process, and while those changes were primarily cosmetic, they happened to also fix this crash bug (which I wasn't aware of until this report). I can either backport the updated patches that are in master and remove the old patches, or I can keep the old patches and just fix this particular crash. If someone has a strong opinion which I should choose, please tell. I personally prefer keeping the old patches, since it's less invasive. Then another question arises: assuming that just a few lines in one of the patches need to be changed, should I replace the buggy patch with a new one, or should I add the fix as a separate patch on top of the old patches? I don't care either way, but unless somebody says something, I will add a separate patch. (In reply to comment #8) > I can make a fix for jethro/krogoth that doesn't require upgrading to 9.0 > and doesn't reintroduce bug 8448. I haven't investigated the crash in much > detail yet, but I would expect the fix to only require changing a few lines > of code. > > The reason why master works is that the patches for bug 8448 had some > significant changes during the upstreaming process, and while those changes > were primarily cosmetic, they happened to also fix this crash bug (which I > wasn't aware of until this report). > > I can either backport the updated patches that are in master and remove the > old patches, or I can keep the old patches and just fix this particular > crash. If someone has a strong opinion which I should choose, please tell. I > personally prefer keeping the old patches, since it's less invasive. Then > another question arises: assuming that just a few lines in one of the > patches need to be changed, should I replace the buggy patch with a new one, > or should I add the fix as a separate patch on top of the old patches? I > don't care either way, but unless somebody says something, I will add a > separate patch. ah, that would be nice. either way is fine for the patch. As I speculated in comment #0, I think the bug should be fairly easy to tickle. However it you have/had any problems reproducing then just ping. I'm happy to test (although after today I shall be away from my desk until 8 Aug). I submitted a fix to upstream (upstream doesn't suffer from the crash, but it turned out that there was some dubious code that together with the other patches caused the crash). In case someone is eager to test it, you can find it here: https://patchwork.freedesktop.org/patch/101926/ I'll be away for a few days. I might be able to finish the work (update the recipes etc.) tomorrow, but if not, I'll do it on Wednesday. I now submitted the fix for jethro and krogoth. jethro: https://patchwork.openembedded.org/patch/128429/ krogoth: https://patchwork.openembedded.org/patch/128431/ |
Preconditions/Environment ------------------------- I have been testing using a Dragonboard 410c (ARM64) and a distro derived from core. However I don't see any reason why this issue could not be reproduced on other boards, architectures or OSs. Distro includes the following packages: pulseaudio pulseaudio-lib-bluez5-util pulseaudio-module-bluetooth-discover pulseaudio-module-bluetooth-policy pulseaudio-module-bluez5-device pulseaudio-module-bluez5-discover pulseaudio-server Triggering Action/Cause ----------------------- Bluetooth disconnect whilst pulseaudio server is running. Expectation ----------- Cards should be torn down without killing the server. Actual Result ------------- I have captures the crash using valgrind and gdb: ~~~ ==2735== Invalid read of size 8 ==2735== at 0x4881654: pa_card_profile_free (card.c:60) ==2735== by 0x498162F: pa_hashmap_remove_all (hashmap.c:230) ==2735== by 0x4981693: pa_hashmap_free (hashmap.c:118) ==2735== by 0x48823BB: pa_card_free (card.c:247) ==2735== by 0xECDFAF7: module_bluez5_device_LTX_pa__done (module-bluez5-device.c:2251) ==2735== by 0x48874BF: pa_module_free (module.c:244) ==2735== by 0xECDD1DF: device_connection_changed_cb (module-bluez5-device.c:2045) ==2735== by 0x4885C0F: pa_hook_fire (hook-list.c:104) ==2735== by 0xE7F491F: pa_bluetooth_transport_unlink (bluez5-util.c:199) ==2735== by 0xE7F9BAF: rfcomm_io_callback (backend-native.c:260) ==2735== by 0x4916A1B: dispatch_pollfds (mainloop.c:655) ==2735== by 0x4916A1B: pa_mainloop_dispatch (mainloop.c:898) ==2735== by 0x4916D9B: pa_mainloop_iterate (mainloop.c:929) ==2735== Address 0xeac4c68 is 88 bytes inside a block of size 112 free'd ==2735== at 0x484411C: free (vg_replace_malloc.c:530) ==2735== by 0x4929E0F: pa_xfree (xmalloc.c:129) ==2735== by 0x48AC677: device_port_free (device-port.c:121) ==2735== by 0x498162F: pa_hashmap_remove_all (hashmap.c:230) ==2735== by 0x4981693: pa_hashmap_free (hashmap.c:118) ==2735== by 0x48823AF: pa_card_free (card.c:244) ==2735== by 0xECDFAF7: module_bluez5_device_LTX_pa__done (module-bluez5-device.c:2251) ==2735== by 0x48874BF: pa_module_free (module.c:244) ==2735== by 0xECDD1DF: device_connection_changed_cb (module-bluez5-device.c:2045) ==2735== by 0x4885C0F: pa_hook_fire (hook-list.c:104) ==2735== by 0xE7F491F: pa_bluetooth_transport_unlink (bluez5-util.c:199) ==2735== by 0xE7F9BAF: rfcomm_io_callback (backend-native.c:260) ==2735== Block was alloc'd at ==2735== at 0x4844E58: calloc (vg_replace_malloc.c:711) ==2735== by 0x4929BB3: pa_xmalloc0 (xmalloc.c:74) ==2735== by 0x488975B: pa_object_new_internal (object.c:41) ==2735== by 0x48ACC3F: pa_device_port_new (device-port.c:132) ==2735== by 0xECDFFD7: create_card_ports (module-bluez5-device.c:1769) ==2735== by 0xECDFFD7: add_card (module-bluez5-device.c:1929) ==2735== by 0xECDFFD7: module_bluez5_device_LTX_pa__init (module-bluez5-device.c:2183) ==2735== by 0x4887BDF: pa_module_load (module.c:180) ==2735== by 0xE7DE403: device_connection_changed_cb (module-bluez5-discover.c:77) ==2735== by 0x4885C0F: pa_hook_fire (hook-list.c:104) ==2735== by 0xE7FA34B: profile_new_connection (backend-native.c:370) ==2735== by 0xE7FA34B: profile_handler (backend-native.c:417) ==2735== by 0x4E0D9CF: _dbus_object_tree_dispatch_and_unlock (dbus-object-tree.c:1020) ==2735== by 0x4DFE5A3: dbus_connection_dispatch (dbus-connection.c:4744) ==2735== by 0x49A5213: dispatch_cb (dbus-util.c:53) ~~~ The following code is the site of the crash. Note that this code was added to solve bug #8448 and is unique to OpenEmbedded. ~~~ 55 56 if (c->ports) { 57 pa_device_port *port; 58 void *state; 59 PA_HASHMAP_FOREACH(port, c->ports, state) 60 pa_hashmap_remove (port->profiles, c->name); 61 pa_hashmap_free(c->ports); 62 } 63 64 pa_xfree(c->input_name); ~~~ For now I have worked around the problem in a fairly brutal fashion by ripping out all patches linked to bug #8448. Reproducibility --------------- Issue reproduces easily on arm64. On this architecture the free() results in the corruption of hashmap function pointers and the PC ends up in the (non-executable) heap. Other architectures may behave differently but valgrind should pick it up pretty much everywhere.