New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak in WiFiManager from Android SDK #106

Closed
johnwatsondev opened this Issue Jan 8, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@johnwatsondev

johnwatsondev commented Jan 8, 2016

  • Instantiation of WifiManager should use Application level Context instead of Activity level in NetworkEvents.java. Otherwise it will occur leak in WifiManager. The reference tree was attached below from leakcanary. This leak was reported in Issue 43945 from android issues.
GC ROOT com.android.internal.util.AsyncChannel$DeathMonitor.this$0
references com.android.internal.util.AsyncChannel.mSrcContext
leaks com.test.demo.SplashActivity instance

Fix code in NetworkEvents.java:

public void register() {
    registerNetworkConnectionChangeReceiver();
    registerInternetConnectionChangeReceiver();

    if (wifiAccessPointsScanEnabled) {
      registerWifiSignalStrengthChangeReceiver();
      // start WiFi scan in order to refresh access point list
      // if this won't be called WifiSignalStrengthChanged may never occur
-     WifiManager wifiManager = (WifiManager) context.getSystemService(Context.WIFI_SERVICE);
+     WifiManager wifiManager = (WifiManager) context.getApplicationContext().getSystemService(Context.WIFI_SERVICE);
      wifiManager.startScan();
    }
  }

  • ConnectivityManager can also leak Activity.

leakcanary dump heap info:

//GC ROOT static android.net.ConnectivityManager.sInstance
//references android.net.ConnectivityManager.mContext
//leaks com.test.demo.SplashActivity instance

Fix code in NetworkConnectionChangeReceiver:

  private ConnectivityStatus getConnectivityStatus(Context context) {
-   ConnectivityManager connectivityManager = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
+   ConnectivityManager connectivityManager = (ConnectivityManager) context.getApplicationContext().getSystemService(Context.CONNECTIVITY_SERVICE);
    NetworkInfo networkInfo = connectivityManager.getActiveNetworkInfo();

    if (networkInfo != null) {
      if (networkInfo.getType() == ConnectivityManager.TYPE_WIFI) {
        return ConnectivityStatus.WIFI_CONNECTED;
      } else if (networkInfo.getType() == ConnectivityManager.TYPE_MOBILE) {
        return ConnectivityStatus.MOBILE_CONNECTED;
      }
    }

    return ConnectivityStatus.OFFLINE;
  }

  • There was an odd behave about those leaks. Because WifiManager and ConnectivityManager were created in so many different places in your codebase. Why just has these two places leak?

Looking forward to your reply. Thanks in advance. :)

@pwittchen

This comment has been minimized.

Show comment
Hide comment
@pwittchen

pwittchen Jan 8, 2016

Owner

Thank you for your work and investigating this issue!

As you can see, constructor of NetworkEvents class accepts Context as a parameter, so you can pass Application Context instead of Activity context inside your app. There's no need to update library.

I'll take a closer look at it later and read note on Google issue tracker carefully to analyze this problem better.

Nevertheless, we can update sample apps in the repository as well as documentation and add appropriate note about this issue in README.md file

Owner

pwittchen commented Jan 8, 2016

Thank you for your work and investigating this issue!

As you can see, constructor of NetworkEvents class accepts Context as a parameter, so you can pass Application Context instead of Activity context inside your app. There's no need to update library.

I'll take a closer look at it later and read note on Google issue tracker carefully to analyze this problem better.

Nevertheless, we can update sample apps in the repository as well as documentation and add appropriate note about this issue in README.md file

@pwittchen pwittchen changed the title from Memory leak to Memory leak in WiFiManager from Android SDK Jan 8, 2016

@johnwatsondev

This comment has been minimized.

Show comment
Hide comment
@johnwatsondev

johnwatsondev Jan 9, 2016

Yep, you got it.

johnwatsondev commented Jan 9, 2016

Yep, you got it.

@pwittchen

This comment has been minimized.

Show comment
Hide comment
@pwittchen

pwittchen Jan 10, 2016

Owner

I've updated code samples and added recommendation note in README.md file in Initialize objects section.

I hope this bug will be fixed in the next Android releases.

I think this issue can be closed for now.

Owner

pwittchen commented Jan 10, 2016

I've updated code samples and added recommendation note in README.md file in Initialize objects section.

I hope this bug will be fixed in the next Android releases.

I think this issue can be closed for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment