caarmen / network-monitor

A network monitor tool for Android that executes an http GET to google.com every x seconds and logs the results into a file.
75 stars 27 forks source link

ca.rmen.android.networkmonitor.app.prefs.AdvancedPreferencesActivity may leak #88

Closed mjblackhorse closed 7 years ago

mjblackhorse commented 7 years ago

I ran the app with a tool based on UiAutomator, and dumped the heap to analyze if there was any leaked activities. Finally I found that instances of "ca.rmen.android.networkmonitor.app.prefs.AdvancedPreferencesActivity" might leak as it they were indirectly referenced by an static field "SERIAL_EXECUTOR" of android.os.AsyncTask, as shown in following figure:

image

Likely, some instances of "ca.rmen.android.networkmonitor.app.speedtest.SpeedTestPreferencesActivity" were indirectly referenced by an static field "SERIAL_EXECUTOR" of android.os.AsyncTask as well :

image

caarmen commented 7 years ago

Thanks for reporting this issue, with the analysis! I'll try to work on a fix soon ☺️

caarmen commented 7 years ago

Could you give me more details on the tool you used?

LeakCanary did not report any leaks. I verified that I was using LeakCanary correctly by temporarily forcing a leak: In one of the activity's onCreate, I added this to a static List<Activity> field, and LeakCanary reported that. But it reported nothing for the AsyncTasks. I verified that the AsyncTasks were run, with breakpoints.

Thanks!

caarmen commented 7 years ago

Even though I saw no leak reported by LeakCanary, I've attempted to adjust the code so that the AsyncTasks will no longer reference the activities. If you get a chance, feel free to rerun your tool on the code again and let me know :)

caarmen commented 7 years ago

The fix is in version 1.28.10.

mjblackhorse commented 7 years ago

Would you please upload the apk for 1.28.10, I can not build the source:(

caarmen commented 7 years ago

oops :) The apks are now in the releases page.

mjblackhorse commented 7 years ago

I've tried the new version with my tool with 1000 actions(while the previous running involved only 500 actions), and I found no non-AOSP leak, although some AOSP leaks exist.

It is likely that the problem has been solved.