1170439 - B2G hangs on the emulator x86 if microphone is muted during answered call

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Emulator, defect)

Description

[AKBNN]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150222232811

Steps to reproduce:

1. Call to the emulator x86 via telnet (gsm call 111333) or ddms.
2. Answer to the incoming call on the emulator x86.
3. Mute microphone.

1. Call to the emulator x86 via telnet (gsm call 111333) or ddms.
2. Answer to the incoming call on the emulator x86.
3. End call by telnet, ddms or the HangUp key.


Actual results:

UI hangs


Expected results:

UI should not hangs.

Comment 1

[Alexander Kazakevich]

B2G hangs after answered call is finished.

Comment 2

[Alexander Kazakevich]

Root cause: Any operation with microphone is blocked beacause the write call is called periodically and mutex for audio_hw object is held constantly.
Proposed solution: Add mutex for the "microphone is muted" flag and use it to set or get this flag here:
B2G/device/generic/goldfish/audio/audio_hw.c

Comment 3

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Hi Bobby,
Can you find someone from audio team to look at this bug?

Comment 4

[Bobby Chien]

Alaster, could you help to handle this? thanks.

Comment 5

[Alastor Wu [:alwu]]

Sure, I'll check it later.
Keep NI for tracking.

Comment 6

[Alastor Wu [:alwu]]

Hi, I think that it's not audio channel issue.
Maybe find someone familiar with RIL or call screen app to check it.

Comment 7

[Alexander Kazakevich]

Attachment: B2G_device_generic_goldfish.patch

Comment 8

[Alexander Kazakevich]

Dear Alastor,

We have investigated this issue and found that B2G is blocked in the "static int adev_get_mic_mute(const struct audio_hw_device *dev, bool *state)" function in file B2G/device/generic/goldfish/audio/audio_hw.c. (We added debug messages).
Muting and unmuting is handled by AudioManager on the emulator (not by RIL).
The same problem is observed when we try to start outgoing call from Dialer App, so it is not the "callscreen" issue.
I think the pthread_mutex_lock does not put waiting threads to a queue, so thread which calls the "write" method unlock and lock mutex in the short period and another thread which waits the same mutex is locked for long period.
The attached patch for "B2G/device/generic/goldfish" seems to have fixed the issue. Could you please check it and if you don’t have any objections, apply it to branch v2.2?

Thank you for attention,
Alexander.

Comment 9

[Alastor Wu [:alwu]]

Sorry, Alex,
Because I don't familiar with these code, I can't review this patch.
You may need other Gonk team guys to review it.
However, for my opinion, I think the double locking is not a good idea.
Maybe we can check the |adev->mic_mute| first, if it's true, we can ignore the |read(...)| and release the lock.

--

Hi, Michael,
Sorry to bother you,
Do you know who can review this patch?
Thanks!

Comment 10

[Josh Cheng [:josh]]

[Blocking Requested - why for this release]:
Continue fixing on next release.

Hi Michael,
Could you help to review the patch per comment 7? Thanks!

Comment 11

[Michael Wu [:mwu]]

Comment on attachment 8628670 [details] [diff] [review]
B2G_device_generic_goldfish.patch

Forwarding to the emulator owner.

Comment 12

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 8628670 [details] [diff] [review]
B2G_device_generic_goldfish.patch

Review of attachment 8628670 [details] [diff] [review]:
-----------------------------------------------------------------

First of all, thanks for the patch!

::: audio/audio_hw.c
@@ +342,3 @@
>      pthread_mutex_lock(&adev->lock);
>      if (adev->fd >= 0)
>          bytes = read(adev->fd, buffer, bytes);

As far as I understand this bug, the problem is that this call to |read| blocks while we are still holding |adev->lock|, right? And there is also an implicit bug in |out_write| where |write| blocks while we're holding this lock.

Instead of introducing another lock, can we just open |adev->fd| with O_NONBLOCK near line 640? If |read| or |write| tells us that (errno == EWOULDBLOCK), we'd return 0 bytes.

Is the caller of this function able to handle this case?

Comment 13

[Alexander Kazakevich]

I have checked proposal solution with the O_NONBLOCK flag. It does not resolve issue.

Comment 14

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 8628670 [details] [diff] [review]
B2G_device_generic_goldfish.patch

Review of attachment 8628670 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the feedback, Alexander.

Let's land the current patch. If the second lock turns out to be a problem, we can still fix it.

Comment 15

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Hi Alex,

if you're actively working an a bug, please assign it to yourself.

If you have a patch with an r+, you're also responsible to landing to it. For Gecko patches, you can do that by setting [checkin-needed] in the Keywords field. For patches against Github repositories, like in this case, you need to create a pull request on Github, get your r+, and set [checkin-needed].

With [checkin-needed], people with commit rights see that the bug report's change is ready for landing and will commit the patch set. Once you fixed a few bugs and people know you, you can get commit rights and land patches yourself.

Comment 16

[Alexey Kistanov]

I'm sorry, but Alex is no longer available. Mozilla guys, could you please take care of delivering this patch (preferably to v2.2r)? Thank you!

Comment 17

[Thomas Zimmermann [:tzimmermann] [:tdz]]

I see. I'll take of landing the patch.

Comment 18

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Alexey,

Which emulator are you using? The bug report says emulator x86, but this doesn't have audio.c. Are you on emulator-x86-kk?

Comment 19

[Alexey Kistanov]

Thomas, yes, it's emulator-x86-kk.

Comment 20

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: Github pull request

Thanks Alexey. Here's the pull request with the patch. Landing this now.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

b2g-4.4.2_r1: https://github.com/mozilla-b2g/device_generic_goldfish/commit/8d4018ebd33ac3f1a043b2d54bc578028656a659