774876 - Toggling off and on the Wifi Checkbox will not scan wifi networks anymore

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Tony Chung [:tchung]]

When disabling wifi checkbox from the settings page, and then re-enabling it again, it will stop scanning and retrieving networks.   Cross filed from bug 772801, which is not a dupe.

Tested against the 7-17-2012 otoro daily build.   

Commit #'s:
  <project name="gaia" path="gaia" remote="b2g" revision="f671a9eec542a460a700a841d1cee72c053d9225"/>
  <project name="mozilla-central" path="gecko" remote="b2g" revision="042312e73785327eb81b7d54346d98b018377060"/>
  <project name="gonk-misc" path="gonk-misc" remote="b2g" revision="cc4675bcaf2ea282c8997785ab7ed006dff927d3"/>
  <project name="rilproxy" path="rilproxy" remote="b2g" revision="32106d4ea635ebe17a1610b643b398db639b8b97"/>


REpro:
1) load settings > wifi
2) uncheck the wifi checkbox  (assuming you had networks found before)
3) recheck the wifi checkbox
4) Verify no networks are ever scanned or found again.

Expected:
- toggling wifi off and on should re-scan wifi networks

Actual:
- toggling wifi off and on does not re-scan wifi networks

Comment 1

[Shian-Yow Wu [:swu]]

This is same as bug 774583.  Per talk with Blake on IRC yesterday, he will look at this bug.

Comment 2

[Kai-Chih Hu [:khu]]

Blake, I heard that you're working on this case. In order to prevent duplicated work, I just assigned this case to you. If you're not working on it right now, I am sorry and please remove your name on it. Thanks!

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

This is a little bizarre. The problem here appears to be a race condition in loading the wifi driver the second time we load it. I can't reproduce the race on the commandline (doing "insmod a; insmod b") but inserting a sleep statement into wifi_load_driver fixes this bug. The result of the race is that both drivers load successfully, however, loading ath6kl_sdio.ko doesn't bring up wlan0 like it does normally.

That being said, Android ICS on the Otoro works because turning off wifi doesn't remove wlan0 at all, so presumably the same race doesn't happen. I haven't been able to figure out why wlan0 doesn't disappear on Android, though. It looks like we call all of the same functions with different results.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Possible fix

I'll keep poking around for another solution, but this patch works.

Comment 6

[Shian-Yow Wu [:swu]]

There was a similar otorol-gb issue (bug 766837), which also related timing issue of wifi driver.
For platform dependent issue, IMHO, if there isn't a general way to fix it in gecko, it is cleaner to fixing it in gonk, even local fork needed.

Comment 7

[Andreas Gal :gal]

Can we simply stop unloading the driver?

Comment 8

[Michael Wu [:mwu]]

(In reply to Andreas Gal :gal from comment #7)
> Can we simply stop unloading the driver?

I had a hack which did exactly that and worked for me.

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Yeah, I have a patch that mostly does that. I'll clean it up and attach it tomorrow.

Comment 10

[Michael Vines [:m1] [:evilmachines]]

FYI, I'm discussing this issue with our partner.  This seems like a better option than a fork of something in gonk.

Comment 11

[Andreas Gal :gal]

Yeah, no gonk forking please.

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Ick

Comment 13

[Michael Wu [:mwu]]

Can we just stop unloading on all devices?

Comment 14

[Vincent Liu[:vliu]]

I am afraid we still have to unload it when the device enter suspend.

Comment 15

[Michael Wu [:mwu]]

(In reply to vliu from comment #14)
> I am afraid we still have to unload it when the device enter suspend.

Why? This is a bug that needs to be fixed on the kernel side if that's the case.

Comment 16

[Andreas Gal :gal]

Does android ever unload? If not neither should we.

Comment 17

[Vincent Liu[:vliu]]

(In reply to Michael Wu [:mwu] from comment #15)
> (In reply to vliu from comment #14)
> > I am afraid we still have to unload it when the device enter suspend.
> 
> Why? This is a bug that needs to be fixed on the kernel side if that's the
> case.

Yes, you are right. Thanks for your correction.

(In reply to Andreas Gal :gal from comment #16)
> Does android ever unload? If not neither should we.

I downloaded Android-ICS on otoro to try. When I tried to toggling on/off wifi, I found wifi kernel module never been unloaded by lsmod checking.

Comment 18

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to vliu from comment #17)
> I downloaded Android-ICS on otoro to try. When I tried to toggling on/off
> wifi, I found wifi kernel module never been unloaded by lsmod checking.

I also found the same thing, but I don't understand it. The code in WifiStateMachine.java tries to unload the driver (see WifiStateMachine.setWifiEnabled) immediately after it shuts down the supplicant. If I remember correctly, the logs also show us entering wifi_unload_driver. What gives?

Comment 19

[Andreas Gal :gal]

Lets land the patch for now as we figure this out. Its an improvement for sure. Should we file a follow-up for the rest?

Comment 20

[Shian-Yow Wu [:swu]]

(In reply to Blake Kaplan (:mrbkap) from comment #18)
> I also found the same thing, but I don't understand it. The code in
> WifiStateMachine.java tries to unload the driver (see
> WifiStateMachine.setWifiEnabled) immediately after it shuts down the
> supplicant. If I remember correctly, the logs also show us entering
> wifi_unload_driver. What gives?
Could be Android-ICS of otoro doesn't really unload driver in wifi_unload_driver()?

Comment 21

[Andreas Gal :gal]

http://hg.mozilla.org/mozilla-central/rev/3613cbdc3481

Comment 22

[Michael Vines [:m1] [:evilmachines]]

Comment on attachment 644564 [details] [diff] [review]
Ick

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

::: dom/wifi/WifiWorker.js
@@ +140,5 @@
>    }
>  
>    function unloadDriver(callback) {
> +    // Otoro ICS can't unload and then load the driver, so never unload it.
> +    if (device === "otoro") {

This expression gets a raspberry from me.  I've captured the need to undo this in https://bugzilla.mozilla.org/show_bug.cgi?id=776311

Comment 23

[Andreas Gal :gal]

Yeah, we should definitely not have this in final. I just need wifi fixed right now. We can iterate.

Comment 24

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Shian-Yow Wu from comment #20)
> Could be Android-ICS of otoro doesn't really unload driver in
> wifi_unload_driver()?

That's possible.

I just noticed that on Android, the driver is unloaded, but wlan0 sticks around in 'netcfg'. I wonder if that might just be a bug in the kernel, though.