Closed Bug 993821 Opened 6 years ago Closed 6 years ago

[Wifi] [Follow up of Bug 986365] Event thread is blocked in WaitForEvent caused wifi stay in searching state

Categories

(Firefox OS Graveyard :: Wifi, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.3+, firefox29 wontfix, firefox30 fixed, firefox31 fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

RESOLVED FIXED
1.4 S5 (11apr)
blocking-b2g 1.3+
Tracking Status
firefox29 --- wontfix
firefox30 --- fixed
firefox31 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: vchang, Assigned: vchang)

References

Details

(Whiteboard: [p=3])

Attachments

(2 files, 1 obsolete file)

No description provided.
Assignee: nobody → vchang
This is found when I doing the gaia ui test. 
STR, 
1. turn on wifi and connect to an AP. 
2. toggle settings to disable wifi. 
3. toggle settings to enabled wifi. 
repeat step 2 and 3 for about 400 times.
Attached patch Patch v1.0 (obsolete) — Splinter Review
The root cause here is
1. When disable wifi, the last event we should receive from wpa_supplicant is 
CTRL-EVENT-TERMINATING. Somehow wpa_supplicant doesn't send it. 
2. It causes event thread waiting in poll function in wifi.c. 
http://androidxref.com/4.0.4/xref/hardware/libhardware_legacy/wifi/wifi.c#662
3. Next time you turn on wifi, the connectToSupplicant command creates new monitor and control sockets and use them to communicate with wpa_supplicant. However, event thread still block on old socket.
Attachment #8403743 - Flags: review?(chulee)
Comment on attachment 8403743 [details] [diff] [review]
Patch v1.0

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

Please refine or remove the debug code, others looks good, thank you!

::: dom/wifi/WifiProxyService.cpp
@@ +16,5 @@
>  using namespace mozilla;
>  using namespace mozilla::dom;
>  
> +#define xxx(args...)  __android_log_print(ANDROID_LOG_INFO, "@@@", args)
> +

nit: "xxx" and "@@@" might not be a proper debug function name/debug message prefix.
If we need to keep these message for debug purpose, we should rename them, add a control flag, and disable them by default.

@@ +202,5 @@
>    mEventThreadList.SetLength(aNumOfInterfaces);
>    for (uint32_t i = 0; i < aNumOfInterfaces; i++) {
>      mEventThreadList[i].mInterface = aInterfaces[i];
>      rv = NS_NewThread(getter_AddRefs(mEventThreadList[i].mThread));
> +    NS_SetThreadName(mEventThreadList[i].mThread, "WifiE");

We usually don't set thread name, and it's seems thread name is not required to resolve the bug.
If we like to keep these names, maybe WifiEventThread/WifiCtrlThread is more friendly, since the max length is 15 chars.
Attachment #8403743 - Flags: review?(chulee) → review+
Address the review comments.
Attachment #8403743 - Attachment is obsolete: true
Attachment #8404496 - Attachment description: Patch v1.1 → Patch v1.1 r=chulee
Attachment #8404496 - Flags: review+
Whiteboard: [p=3]
Target Milestone: --- → 1.4 S6 (25apr)
Blocks: 986365
Summary: [Wifi] Event thread is blocked in WaitForEvent caused wifi stay in searching state → [Wifi] [Follow up of Bug 986365] Event thread is blocked in WaitForEvent caused wifi stay in searching state
Nominate for 1.3 ? I found this when doing the test after apply the patch in Bug 986365 which is a 1.3 blocker. We should apply this to prevent breaking wifi function.
blocking-b2g: --- → 1.3?
upgrading this to 1.4+.   1.3 branch is not getting any new automation at this point.   please land this on 1.4 branch, thanks!
blocking-b2g: 1.3? → 1.4+
https://hg.mozilla.org/mozilla-central/rev/ce9320e85ba8
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
(In reply to Vincent Chang[:vchang] from comment #6)
> Nominate for 1.3 ? I found this when doing the test after apply the patch in
> Bug 986365 which is a 1.3 blocker. We should apply this to prevent breaking
> wifi function.

(In reply to Tony Chung [:tchung] from comment #8)
> upgrading this to 1.4+.   1.3 branch is not getting any new automation at
> this point.   please land this on 1.4 branch, thanks!

Making this a 1.3+  based on comment #6. IN short we have to uplift https://bugzilla.mozilla.org/show_bug.cgi?id=986365 and by doing so we may break wifi unless we uplift this.
blocking-b2g: 1.4+ → 1.3+
Please request b2g28 approval for v1.3 uplift on this ASAP.
Flags: needinfo?(vchang)
Comment on attachment 8404517 [details] [diff] [review]
Patch v1.1 for 1.3 branch

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

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This is found after applied patch in Bug 986365.
User impact if declined: Toggle wifi settings on/off for about 4 hours causes event thread blocked, we need to reboot the device to make wifi work again. 
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): No
String or IDL/UUID changes made by this patch: No
Attachment #8404517 - Flags: approval-mozilla-b2g28?
Flags: needinfo?(vchang)
Attachment #8404517 - Flags: approval-mozilla-b2g28? → approval-mozilla-b2g28+
You need to log in before you can comment on or make changes to this bug.