Fix leaks by SUPERCILEX · Pull Request #429 · firebase/FirebaseUI-Android

SUPERCILEX · 2016-12-01T05:35:08Z

I found another leak related to Twitter in AuthMethodPickerActivity:

In com.firebase.uidemo:1.0:1.
* com.firebase.ui.auth.ui.idp.AuthMethodPickerActivity has leaked:
* GC ROOT static io.fabric.sdk.android.Fabric.singleton
* references io.fabric.sdk.android.Fabric.idManager
* references io.fabric.sdk.android.services.common.IdManager.appContext
* leaks com.firebase.ui.auth.ui.idp.AuthMethodPickerActivity instance
* Retaining: 109 KB.
* Reference Key: 99395659-f480-46ab-a1fa-26e46beec10e
* Device: Google google Pixel XL marlin
* Android Version: 7.1 API: 25 LeakCanary: 1.5 00f37f5
* Durations: watch=5165ms, gc=170ms, heap dump=18027ms, analysis=166278ms

* Details:
* Class io.fabric.sdk.android.Fabric
|   static TAG = java.lang.String@317782464 (0x12f0f9c0)
|   static $classOverhead = byte[233]@317961713 (0x12f3b5f1)
|   static DEFAULT_DEBUGGABLE = false
|   static ROOT_DIR = java.lang.String@317782432 (0x12f0f9a0)
|   static singleton = io.fabric.sdk.android.Fabric@318157032 (0x12f6b0e8)
|   static DEFAULT_LOGGER = io.fabric.sdk.android.DefaultLogger@317884032 (0x12f28680)
* Instance of io.fabric.sdk.android.Fabric
|   static TAG = java.lang.String@317782464 (0x12f0f9c0)
|   static $classOverhead = byte[233]@317961713 (0x12f3b5f1)
|   static DEFAULT_DEBUGGABLE = false
|   static ROOT_DIR = java.lang.String@317782432 (0x12f0f9a0)
|   static singleton = io.fabric.sdk.android.Fabric@318157032 (0x12f6b0e8)
|   static DEFAULT_LOGGER = io.fabric.sdk.android.DefaultLogger@317884032 (0x12f28680)
|   activity = java.lang.ref.WeakReference@317485984 (0x12ec73a0)
|   activityLifecycleManager = io.fabric.sdk.android.ActivityLifecycleManager@317884560 (0x12f28890)
|   context = com.firebase.uidemo.auth.LeakCatcher@315064512 (0x12c780c0)
|   debuggable = false
|   executorService = io.fabric.sdk.android.services.concurrency.PriorityThreadPoolExecutor@317439496 (0x12ebbe08)
|   idManager = io.fabric.sdk.android.services.common.IdManager@317786336 (0x12f108e0)
|   initializationCallback = io.fabric.sdk.android.InitializationCallback$Empty@315847600 (0x12d373b0)
|   initialized = java.util.concurrent.atomic.AtomicBoolean@317884528 (0x12f28870)
|   kitInitializationCallback = io.fabric.sdk.android.Fabric$2@317887896 (0x12f29598)
|   kits = java.util.HashMap@317766040 (0x12f0b998)
|   logger = io.fabric.sdk.android.DefaultLogger@317884224 (0x12f28740)
|   mainHandler = android.os.Handler@317782752 (0x12f0fae0)
|   shadow$_klass_ = io.fabric.sdk.android.Fabric
|   shadow$_monitor_ = 0
* Instance of io.fabric.sdk.android.services.common.IdManager
|   static ID_PATTERN = java.util.regex.Pattern@317887320 (0x12f29358)
|   static BAD_ANDROID_ID = java.lang.String@317786240 (0x12f10880)
|   static COLLECT_USER_IDENTIFIERS = java.lang.String@317677376 (0x12ef5f40)
|   static PREFKEY_INSTALLATION_UUID = java.lang.String@317439568 (0x12ebbe50)
|   static FORWARD_SLASH_REGEX = java.lang.String@317782848 (0x12f0fb40)
|   static $classOverhead = byte[284]@317335937 (0x12ea2981)
|   static COLLECT_DEVICE_IDENTIFIERS = java.lang.String@317677280 (0x12ef5ee0)
|   static DEFAULT_VERSION_NAME = java.lang.String@1871495192 (0x6f8cc018)
|   advertisingInfo = io.fabric.sdk.android.services.common.AdvertisingInfo@318235536 (0x12f7e390)
|   advertisingInfoProvider = io.fabric.sdk.android.services.common.AdvertisingInfoProvider@317884384 (0x12f287e0)
|   appContext = com.firebase.ui.auth.ui.idp.AuthMethodPickerActivity@317466768 (0x12ec2890)
|   appIdentifier = java.lang.String@314718184 (0x12c237e8)
|   appInstallIdentifier = null
|   collectHardwareIds = true
|   collectUserIds = true
|   fetchedAdvertisingInfo = true
|   installationIdLock = java.util.concurrent.locks.ReentrantLock@317884320 (0x12f287a0)
|   installerPackageNameProvider = io.fabric.sdk.android.services.common.InstallerPackageNameProvider@317884336 (0x12f287b0)
|   kits = java.util.HashMap$Values@317884304 (0x12f28790)
|   shadow$_klass_ = io.fabric.sdk.android.services.common.IdManager
|   shadow$_monitor_ = 0
* Instance of com.firebase.ui.auth.ui.idp.AuthMethodPickerActivity
|   static $classOverhead = byte[3980]@317562881 (0x12eda001)
|   static RC_ACCOUNT_LINK = 3
|   static TAG = java.lang.String@317102016 (0x12e697c0)
|   static RC_EMAIL_FLOW = 2
|   mIdpProviders = java.util.ArrayList@317705784 (0x12efce38)
|   mSaveSmartLock = com.firebase.ui.auth.util.signincontainer.SaveSmartLock@316057776 (0x12d6a8b0)
|   mActivityHelper = com.firebase.ui.auth.ui.ActivityHelper@317474320 (0x12ec4610)
|   mDelegate = android.support.v7.app.AppCompatDelegateImplN@317551520 (0x12ed73a0)
|   mEatKeyUpEvent = false
|   mResources = null
|   mThemeId = 2131427471
|   mCreated = true
|   mFragments = android.support.v4.app.FragmentController@317226112 (0x12e87c80)
|   mHandler = android.support.v4.app.FragmentActivity$1@317571424 (0x12edc160)
|   mMediaController = null
|   mNextCandidateRequestIndex = 0
|   mOptionsMenuInvalidated = false
|   mPendingFragmentActivityResults = android.support.v4.util.SparseArrayCompat@317474296 (0x12ec45f8)
|   mReallyStopped = true
|   mRequestedPermissionsFromFragment = false
|   mResumed = false
|   mRetaining = false
|   mStopped = true
|   mStartedActivityFromFragment = false
|   mStartedIntentSenderFromFragment = false
|   mActionBar = null
|   mActionModeTypeStarting = 0
|   mActivityInfo = android.content.pm.ActivityInfo@317551376 (0x12ed7310)
|   mActivityTransitionState = android.app.ActivityTransitionState@317393152 (0x12eb0900)
|   mApplication = com.firebase.uidemo.auth.LeakCatcher@315064512 (0x12c780c0)
|   mCalled = true
|   mChangeCanvasToTranslucent = false
|   mChangingConfigurations = false
|   mComponent = android.content.ComponentName@317199600 (0x12e814f0)
|   mConfigChangeFlags = 0
|   mCurrentConfig = android.content.res.Configuration@317423096 (0x12eb7df8)
|   mDecor = null
|   mDefaultKeyMode = 0
|   mDefaultKeySsb = null
|   mDestroyed = true
|   mDoReportFullyDrawn = false
|   mEatKeyUpEvent = false
|   mEmbeddedID = null
|   mEnableDefaultActionBarUp = false
|   mEnterTransitionListener = android.app.SharedElementCallback$1@1877714360 (0x6feba5b8)
|   mExitTransitionListener = android.app.SharedElementCallback$1@1877714360 (0x6feba5b8)
|   mFinished = true
|   mFragments = android.app.FragmentController@317226064 (0x12e87c50)
|   mHandler = android.os.Handler@317571360 (0x12edc120)
|   mHasCurrentPermissionsRequest = false
|   mIdent = 25238395
|   mInstanceTracker = android.os.StrictMode$InstanceTracker@317226080 (0x12e87c60)
|   mInstrumentation = android.app.Instrumentation@314721328 (0x12c24430)
|   mIntent = android.content.Intent@317534384 (0x12ed30b0)
|   mLastNonConfigurationInstances = null
|   mMainThread = android.app.ActivityThread@314589440 (0x12c04100)
|   mManagedCursors = java.util.ArrayList@317473744 (0x12ec43d0)
|   mManagedDialogs = null
|   mMenuInflater = null
|   mParent = null
|   mReferrer = java.lang.String@317534832 (0x12ed3270)
|   mResultCode = 0
|   mResultData = null
|   mResumed = false
|   mSearchEvent = null
|   mSearchManager = null
|   mStartedActivity = false
|   mStopped = true
|   mTaskDescription = android.app.ActivityManager$TaskDescription@317571392 (0x12edc140)
|   mTemporaryPause = false
|   mTitle = java.lang.String@315068712 (0x12c79128)
|   mTitleColor = 0
|   mTitleReady = true
|   mToken = android.os.BinderProxy@317399392 (0x12eb2160)
|   mTranslucentCallback = null
|   mUiThread = java.lang.Thread@1956798296 (0x74a25f58)
|   mVisibleBehind = false
|   mVisibleFromClient = true
|   mVisibleFromServer = true
|   mVoiceInteractor = null
|   mWindow = com.android.internal.policy.PhoneWindow@316112544 (0x12d77ea0)
|   mWindowAdded = true
|   mWindowManager = android.view.WindowManagerImpl@317474056 (0x12ec4508)
|   mInflater = com.android.internal.policy.PhoneLayoutInflater@317100768 (0x12e692e0)
|   mOverrideConfiguration = null
|   mResources = android.content.res.Resources@317304112 (0x12e9ad30)
|   mTheme = android.content.res.Resources$Theme@317226256 (0x12e87d10)
|   mThemeResource = 2131427471
|   mBase = android.app.ContextImpl@317424352 (0x12eb82e0)
|   shadow$_klass_ = com.firebase.ui.auth.ui.idp.AuthMethodPickerActivity
|   shadow$_monitor_ = 1073744196
* Excluded Refs:
| Field: android.view.Choreographer$FrameDisplayEventReceiver.mMessageQueue (always)
| Thread:FinalizerWatchdogDaemon (always)
| Thread:main (always)
| Thread:LeakCanary-Heap-Dump (always)
| Class:java.lang.ref.WeakReference (always)
| Class:java.lang.ref.SoftReference (always)
| Class:java.lang.ref.PhantomReference (always)
| Class:java.lang.ref.Finalizer (always)
| Class:java.lang.ref.FinalizerReference (always)

According to the Twitter and Facebook documentation, we weren't using the right context.

Once #418 gets merged, all leaks I've noticed will have been patched!

samtstern · 2016-12-01T17:41:01Z

Thanks for this! Nothing wrong with the fix as it stands but I think the safer way to do this is to have the TwitterProvider and FacebookProvider classes always call getApplicationContext() on the Context arguments they receive in their constructor. That way we don't actually cause this same leak again in the future.

SUPERCILEX

Ah, that's a much better way of doing it! (I'm complaining about twitter not having a good API and here I am doing the same thing! 😅)

SUPERCILEX · 2016-12-01T17:47:32Z

auth/src/main/java/com/firebase/ui/auth/provider/FacebookProvider.java

    private IdpCallback mCallbackObject;

    public FacebookProvider(Context appContext, IdpConfig idpConfig) {
+        appContext = appContext.getApplicationContext();


Do we want to do this or only call getApplicationContext when we are passing it into the Facebook initializer?

This looks good to me. Either way would have been fine.

samtstern · 2016-12-01T17:54:03Z

Merging!

SUPERCILEX added 2 commits November 30, 2016 21:23

Fix Twitter leak and possible Facebook leak

5c87295

Fix possible related leaks

0a06297

Use black box principles

273181f

SUPERCILEX commented Dec 1, 2016

View reviewed changes

samtstern merged commit 7f2ef5f into firebase:master Dec 1, 2016

SUPERCILEX deleted the fix-leaks branch December 1, 2016 17:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix leaks#429

Fix leaks#429
samtstern merged 3 commits intofirebase:masterfrom
SUPERCILEX:fix-leaks

SUPERCILEX commented Dec 1, 2016

Uh oh!

samtstern commented Dec 1, 2016 •

edited

Loading

Uh oh!

SUPERCILEX left a comment

Uh oh!

SUPERCILEX Dec 1, 2016

Uh oh!

samtstern Dec 1, 2016

Uh oh!

samtstern commented Dec 1, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

SUPERCILEX commented Dec 1, 2016

Uh oh!

samtstern commented Dec 1, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SUPERCILEX left a comment

Choose a reason for hiding this comment

Uh oh!

SUPERCILEX Dec 1, 2016

Choose a reason for hiding this comment

Uh oh!

samtstern Dec 1, 2016

Choose a reason for hiding this comment

Uh oh!

samtstern commented Dec 1, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

samtstern commented Dec 1, 2016 •

edited

Loading