1152272 - [RIL] emulator's rild suddenly stop processing the request when running test_mobile_set_radio.js

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Emulator, defect)

Description

[Edgar Chen [:edgar]]

emulator's rild suddenly stop processing the request when running test_mobile_set_radio.js, so all subsequence ril related marionette tests are timed out.

https://treeherder.mozilla.org/logviewer.html#?job_id=6213755&repo=try
https://treeherder.mozilla.org/logviewer.html#?job_id=6213771&repo=try
https://treeherder.mozilla.org/logviewer.html#?job_id=6213879&repo=try

Comment 1

[Edgar Chen [:edgar]]

I suspect it is same issue as https://bugzilla.mozilla.org/show_bug.cgi?id=1018051#c0.

Comment 2

[Edgar Chen [:edgar]]

As mention in https://bugzilla.mozilla.org/show_bug.cgi?id=1018051#c0, there is only one buffer [1] to catch the AT command , but it will be accessed by follow two threads,
- thread for processing console command. (some console command will trigger sending unsolicited at command)
- thread for processing request from rild.

The data in buffer written by a thread could be corrupted by the other thread.
The idea solution is adding mutex to protect the buffer.

[1] https://github.com/mozilla-b2g/platform_external_qemu/blob/d9fbd6c2a014f52ab64e8ca5cb8ace8947c7e6e4/telephony/android_modem.c#L409

Comment 3

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

[Tracking Requested - why for this release]:

Comment 4

[Edgar Chen [:edgar]]

Working branch: https://github.com/EdgarChen/platform_external_qemu/commits/bug_1152272/mutex

Comment 5

[Edgar Chen [:edgar]]

(In reply to Edgar Chen [:edgar][:echen] from comment #4)
> Working branch:
> https://github.com/EdgarChen/platform_external_qemu/commits/bug_1152272/mutex

Try runs with fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5fcf944c3370&exclusion_profile=false

Comment 6

[Edgar Chen [:edgar]]

(In reply to Edgar Chen [:edgar][:echen] from comment #5)
> (In reply to Edgar Chen [:edgar][:echen] from comment #4)
> > Working branch:
> > https://github.com/EdgarChen/platform_external_qemu/commits/bug_1152272/mutex
> 
> Try runs with fix:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=5fcf944c3370&exclusion_profile=false

Didn't see same failure in test_mobile_set_radio.js so far. (over 80+ retriggers)

Comment 7

[Edgar Chen [:edgar]]

Attachment: [platform_external_qemu] PR #138

Comment 8

[Edgar Chen [:edgar]]

Comment on attachment 8593893 [details] [review]
[platform_external_qemu] PR #138

Hi Hsinyi, I did some refactor for AT command handler and also added mutex to protect the out buffer in this PR, would you mind reviewing this? Thank you.

Try runs with this fix and also bug 1155022 for your reference: 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=74365e5cb51a&exclusion_profile=false

Comment 9

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8593893 [details] [review]
[platform_external_qemu] PR #138

You could find my comments on github.

Comment 10

[Edgar Chen [:edgar]]

Thank you, Hsinyi.

https://github.com/mozilla-b2g/platform_external_qemu/commit/8c2d32bccc7061e9ca0165135457c3fd53e7107e