CRAS: crash in stream_added_cb |
|||||||||
Issue descriptionhttps://crash.corp.google.com/browse?q=reportid=%27228f7f95a3495247%27#0 Stack 0x00005b3fc7ff9182 (cras -cras_apm_list.c:261 ) cras_apm_list_add 0x00005b3fc800be58 (cras -cras_iodev_list.c:648 ) stream_added_cb 0x00005b3fc80194db (cras -stream_list.c:88 ) stream_list_add 0x00005b3fc8011543 (cras -cras_rclient.c:74 ) handle_client_stream_connect 0x00005b3fc8010d32 (cras -cras_rclient.c ) cras_rclient_message_from_client 0x00005b3fc8010c5d (cras -cras_rclient.c:421 ) cras_rclient_buffer_from_client 0x00005b3fc7feac4a (cras -cras_server.c:135 ) cras_server_run 0x00005b3fc7fea11d (cras -cras.c:141 ) main 0x00007e63495f9735 (libc-2.23.so -libc-start.c:289 ) __libc_start_main 0x00005b3fc7fe9d28 (cras + 0x00007d28 ) _start 0x00007fff27eba637 0x00005b3fc7fe9cff (cras + 0x00007cff ) _init
,
Nov 15
Some more info, this is an input stream pinning to dev id 13, and enables system AEC. Highly suspect it's from Meets app, using javascript API to select to BT/USB device.
$10 = {stream_id = 1572864, stream_type = CRAS_STREAM_TYPE_DEFAULT, direction = CRAS_STREAM_INPUT, dev_idx = 13, flags = 0, effects = 1, format = 0x7fff27eba0b0, buffer_frames = 480, cb_threshold = 480, audio_fd = 27, client = 0x5b3fc8a752f0}
(gdb) p stream_config.format
$11 = (const struct cras_audio_format *) 0x7fff27eba0b0
(gdb) p *stream_config.format
$12 = {format = SND_PCM_FORMAT_S16_LE, frame_rate = 48000, num_channels = 2, channel_layout = "\000\001\377\377\377\377\377\377\377\377\377"}
(gdb)
,
Nov 16
,
Nov 16
Finally found the root cause. Credits to recent change that enables system AEC for nocturne, which gives me constantly repro steps, as follow: (1) On nocturne, connect power beats BT headset. (2) Launch http://meet.google.com/_meet (3) Click top right corner gear to open in-app audio settings, select both input/output option to Powerbeats. At (3) Meets app creates input stream that pins to BT headset, and forces it to switch from A2DP to HFP. Profile switching throws -EAGAIN code at bt_io open. Since we whitelisted this err code in init_pinned_device(), iodev_list will proceed to pass this bt_io to audio_thread even it hasn't actually open. As I mentioned earlier, nocturne by default uses system AEC. And that moves the crash to happen earlier in main thread when using iodev's format to create APM. Note that on the other boards without default system AEC, this crash is hard to analyze. It's because the NULL format is used only in audio thread, when a stream appends to iodev. And that happens after BT profiles switched and got a valid format, according to below events(ordered in time). (a) bt iodev open (which fails, but we didn't treat it as error) (b) Add bt iodev to audio thread (c) bt iodev profile switch (d) Append stream to bt iodev Uploaded two CLs, one is to fix the problem and the 2nd one to catch & log a rare but possible case. https://crrev.com/c/1339600 CRAS: iodev_list - Fix err handling for pinned stream https://crrev.com/c/1337236 CRAS: iodev_list - Fix potential crash when init pinned stream
,
Nov 19
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/adhd/+/dcdf22084db56acf462466c329e6939c3f631b0f commit dcdf22084db56acf462466c329e6939c3f631b0f Author: Hsin-Yu Chao <hychao@google.com> Date: Mon Nov 19 03:49:44 2018 CRAS: iodev_list - Fix err handling for pinned stream CRAS treats EAGAIN error code differently at init pinned device and that leaks iodev to audio thread without being open. The EAGAIN error comes from BT type iodev, when input stream connected but headset hasn't switched to HFP. This causes a bunch of crashes at accessing iodev's format before headset completes profile switching. It's totally okay to remove this EAGAIN handling now to fix the crashes. Because we have init retry mechanism added in commit 380919c44 to handle the asynchronous profile switching. BUG= chromium:905580 TEST=Use Meets on nocturne, connect BT headset, open in-app audio settings and select both input/output to BT headset. Verify there's no crash. Change-Id: I25795f86a314e5a81d54e7fe1804a341e9d5d646 Reviewed-on: https://chromium-review.googlesource.com/1339600 Commit-Ready: Hsinyu Chao <hychao@chromium.org> Tested-by: Hsinyu Chao <hychao@chromium.org> Reviewed-by: Dylan Reid <dgreid@chromium.org> [modify] https://crrev.com/dcdf22084db56acf462466c329e6939c3f631b0f/cras/src/server/cras_iodev_list.c
,
Nov 19
Request merge to 71 for the CL in #5 Note: this is verified on Tot CRAS, according to analysis in #4 and TEST instructions in the commit message.
,
Nov 19
,
Nov 19
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 19
Approved for M71 ChromeOS
,
Nov 20
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/adhd/+/017cccd7cb89039ec12269c8351ea07514779b7a commit 017cccd7cb89039ec12269c8351ea07514779b7a Author: Hsin-Yu Chao <hychao@google.com> Date: Tue Nov 20 03:24:29 2018 CRAS: iodev_list - Fix err handling for pinned stream CRAS treats EAGAIN error code differently at init pinned device and that leaks iodev to audio thread without being open. The EAGAIN error comes from BT type iodev, when input stream connected but headset hasn't switched to HFP. This causes a bunch of crashes at accessing iodev's format before headset completes profile switching. It's totally okay to remove this EAGAIN handling now to fix the crashes. Because we have init retry mechanism added in commit 380919c44 to handle the asynchronous profile switching. BUG= chromium:905580 TEST=Use Meets on nocturne, connect BT headset, open in-app audio settings and select both input/output to BT headset. Verify there's no crash. Change-Id: I25795f86a314e5a81d54e7fe1804a341e9d5d646 Reviewed-on: https://chromium-review.googlesource.com/1339600 Commit-Ready: Hsinyu Chao <hychao@chromium.org> Tested-by: Hsinyu Chao <hychao@chromium.org> Reviewed-by: Dylan Reid <dgreid@chromium.org> (cherry picked from commit dcdf22084db56acf462466c329e6939c3f631b0f) Reviewed-on: https://chromium-review.googlesource.com/c/1343698 Reviewed-by: Hsinyu Chao <hychao@chromium.org> Commit-Queue: Hsinyu Chao <hychao@chromium.org> [modify] https://crrev.com/017cccd7cb89039ec12269c8351ea07514779b7a/cras/src/server/cras_iodev_list.c
,
Nov 20
Let's see how the crashes improve in M71.
,
Nov 20
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by hychao@chromium.org
, Nov 15