Skip to content
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

LeakCanary investigation to find memory leaks #395

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

archie94
Copy link
Collaborator

@archie94 archie94 commented Apr 21, 2019

Call cameraview destroy() instead of close() on Fragment onDestroy() Do the same with LifecycleOwner -> #393

- destroy() releases the camera resource unlike close() which just stops the preview

Signed-off-by: Arka Prava Basu <[email protected]>
- setting LifecycleOwner will remove boilerplate around open(),
  close() and destroy()
- a possible fix for guardianproject#376 ?

Signed-off-by: Arka Prava Basu <[email protected]>
- holding this reference may lead to Context leak

Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
- keeping Context (Activity/Fragment) in a singleton will cause memory leaks
  Keep the global application object associated with it instead
- fix guardianproject#399

Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
- Among others this updates materialdatetimepicker to 4.2.0 which
  fixes a memory leak

Signed-off-by: Arka Prava Basu <[email protected]>
- keep this app dependency free; can be added in debug builds whenever
  another investigation is required

Signed-off-by: Arka Prava Basu <[email protected]>
@archie94 archie94 changed the title WIP - LeakCanary investigation to find memory leaks LeakCanary investigation to find memory leaks May 14, 2019
@archie94
Copy link
Collaborator Author

Some major leaks around Camera, Settings, Signal and MaterialDateTime Picker fixed. Please test once and consider merging.

@n8fr8
Copy link
Member

n8fr8 commented May 29, 2019

Merged fine, and is not crashing, but seem to not be capturing all the camera positive detect events. Will dig and test some more.

@n8fr8
Copy link
Member

n8fr8 commented May 29, 2019

Definitely something different happening with logging events. Master works fine, but this PR seems to not be persisting matched events.

@archie94
Copy link
Collaborator Author

:/
Will check it out! Meanwhile if you feel it is happening for any specific scenario or find any steps to reproduce do leave them here.

I hope these event / event triggers are solely around the camera?

Thanks for the testing!

Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
- aims to solve guardianproject#395 (comment)
- According to https://developer.android.com/reference/androidx/lifecycle/DefaultLifecycleObserver
  [DefaultLifecycleObserver] should *always* be preferred over [androidx.lifecycle.LifecycleObserver]
  if we use Java 8. [CameraView] library targets Java 7 hence this implementation aims to ignore
  [androidx.lifecycle.OnLifecycleEvent] annotated methods in the super class and replace them with
  the callbacks implemeted in this sub class

Signed-off-by: Arka Prava Basu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MonitorActivity Leak
2 participants