fix memory leak

[lzdon]

sorry , i forgot validation

[lzdon]

why always check failed ? I just add one line code

[Serchinastico]

Hey @lzdon

I'm taking a look at the failing test and I'd say the test is incomplete as it stands right now, looks like we are not setting what to do in DexterInstance::336:

int permissionState = androidPermissionService.checkSelfPermission(context, permission);

and therefore it's returning 0 (default value), however, by the name of the test I'd say it should return PERMISSION_DENIED = -1 to then try with the invisible activity to do the same check and failing to do so. Here is the patch to fix it so that we can merge your PR:

  @ -132,6 +129,7 @@ import static org.mockito.Mockito.when;
    }

    @Test public void onPermissionFailedByRuntimeExceptionThenNotifiesListener() {
+     givenPermissionIsChecked(ANY_PERMISSION, PackageManager.PERMISSION_DENIED);
      givenARuntimeExceptionIsThrownWhenPermissionIsChecked(ANY_PERMISSION);
      givenShouldShowRationaleForPermission(ANY_PERMISSION);

[sklinefelter]

#197 Unfortunately I just tested this locally and still see LeakCanary indicating the listener leaks the activity.

I was able to reproduce by adding LeakCanary to the Dexter sample and rotating once the permissions dialog was shown.

My suspicion is it's related to Multiple-Permissions

[Serchinastico]

Hi @sklinefelter

Do you have the hprof file? I've been testing for a while and I can't reproduce the issue, what's more, at first LeakCanary detected something but in the end it just reported that The GC was being lazy (See square/leakcanary#536).

[sklinefelter]

I do have the additional details and can reproduce consistently.

Steps to reproduce:

1. Add LeakCanary to a basic SampleApplication

public class SampleApplication extends Application {

    @Override
    public void onCreate() {
        super.onCreate();
        if (LeakCanary.isInAnalyzerProcess(this)) {
            // This process is dedicated to LeakCanary for heap analysis.
            // You should not init your app in this process.
            return;
        }
        LeakCanary.install(this);
    }
}

2. Adjust the AndroidManifest to have the ".SampleApplication"

  <application
      android:allowBackup="true"
      android:label="@string/app_name"
      android:icon="@mipmap/ic_logo_karumi"
      android:supportsRtl="true"
      android:theme="@style/AppTheme"
      android:name=".SampleApplication">

3. Accept storage permssion (this gets past the "GC was being lazy" issue for me somehow)
4. Open the permissions dialog
5. Rotate the phone (sometimes all it takes is one, sometimes you have to rotate 3 or 4 times)

Eventually, the LeakCanary notification comes through

Here's the leak:

In com.karumi.dexter.sample:1.0:1.

• com.karumi.dexter.sample.SampleActivity has leaked:
• GC ROOT android.os.HandlerThread.contextClassLoader
• references dalvik.system.PathClassLoader.runtimeInternalObjects
• references array java.lang.Object[].[10]
• references static com.karumi.dexter.Dexter.instance
• references com.karumi.dexter.DexterInstance.listener
• references com.karumi.dexter.MultiplePermissionListenerThreadDecorator.listener
• references com.karumi.dexter.listener.multi.CompositeMultiplePermissionsListener.listeners
• references java.util.Arrays$ArrayList.a
• references array com.karumi.dexter.listener.multi.MultiplePermissionsListener[].[0]
• references com.karumi.dexter.sample.SampleMultiplePermissionListener.activity
• leaks com.karumi.dexter.sample.SampleActivity instance
• Retaining: 1.4 kB.
• Reference Key: 58047889-7492-429e-9f11-fcefe81f7ecc
• Device: Google google Pixel sailfish
• Android Version: 8.1.0 API: 27 LeakCanary: 1.5.4 74837f0
• Durations: watch=5544ms, gc=119ms, heap dump=1804ms, analysis=49174ms
• Details:
• Instance of android.os.HandlerThread
  | static $class$methods = 1901801520
  | static $class$vtable = null
  | static $class$numReferenceStaticFields = 0
  | static $class$clinitThreadId = 0
  | static $classOverhead = byte[420]@1898567041 (0x7129d581)
  | static $class$dexTypeIndex = 5111
  | static $class$numReferenceInstanceFields = 2
  | static $class$virtualMethodsOffset = 2
  | static $class$classLoader = null
  | static $class$dexCache = java.lang.DexCache@1897028624 (0x71125c10)
  | static $class$iFields = 1900117996
  | static $class$shadow$klass = java.lang.Class
  | static $class$dexClassDefIndex = 2731
  | static $class$name = java.lang.String@1899490136 (0x7137eb58)
  | static $class$accessFlags = 524289
  | static $class$classSize = 544
  | static $class$sFields = 0
  | static $class$primitiveType = 131072
  | static $class$objectSize = 148
  | static $class$objectSizeAllocFastPath = 152
  | static $class$referenceInstanceOffsets = -1073741824
  | static $class$componentType = null
  | static $class$ifTable = java.lang.Object[2]@1897090168 (0x71134c78)
  | static $class$classFlags = 0
  | static $class$extData = null
  | static $class$shadow$monitor = 536870912
  | static $class$copiedMethodsOffset = 9
  | static $class$status = 11
  | static $class$superClass = java.lang.Thread
  | mHandler = null
  | mLooper = android.os.Looper@315101744 (0x12c81230)
  | mPriority = 0
  | mTid = 6342
  | blocker = null
  | blockerLock = java.lang.Object@315504056 (0x12ce35b8)
  | contextClassLoader = dalvik.system.PathClassLoader@315150696 (0x12c8d168)
  | daemon = false
  | eetop = 0
  | group = java.lang.ThreadGroup@1892864888 (0x70d2d378)
  | inheritableThreadLocals = null
  | inheritedAccessControlContext = java.security.AccessControlContext@315504064 (0x12ce35c0)
  | lock = java.lang.Object@315504072 (0x12ce35c8)
  | name = java.lang.String@315155176 (0x12c8e2e8)
  | nativeParkEventPointer = 0
  | nativePeer = 505793973760
  | parkBlocker = null
  | parkState = 1
  | priority = 5
  | single_step = false
  | stackSize = 0
  | started = true
  | stillborn = false
  | target = null
  | threadLocalRandomProbe = 0
  | threadLocalRandomSecondarySeed = 0
  | threadLocalRandomSeed = 0
  | threadLocals = java.lang.ThreadLocal$ThreadLocalMap@315504080 (0x12ce35d0)
  | threadQ = null
  | threadStatus = 0
  | tid = 475
  | uncaughtExceptionHandler = null
  | shadow$klass = android.os.HandlerThread
  | shadow$monitor = 0
• Instance of dalvik.system.PathClassLoader
  | static $class$methods = 1895576376
  | static $class$vtable = null
  | static $class$numReferenceStaticFields = 0
  | static $class$clinitThreadId = 0
  | static $classOverhead = byte[316]@1895263521 (0x70f76d21)
  | static $class$dexTypeIndex = 1264
  | static $class$numReferenceInstanceFields = 0
  | static $class$virtualMethodsOffset = 2
  | static $class$classLoader = null
  | static $class$dexCache = java.lang.DexCache@1895182352 (0x70f63010)
  | static $class$iFields = 0
  | static $class$shadow$klass = java.lang.Class
  | static $class$dexClassDefIndex = 1244
  | static $class$name = java.lang.String@1899563328 (0x71390940)
  | static $class$accessFlags = 524289
  | static $class$classSize = 440
  | static $class$sFields = 0
  | static $class$primitiveType = 131072
  | static $class$objectSize = 44
  | static $class$objectSizeAllocFastPath = 48
  | static $class$referenceInstanceOffsets = 263
  | static $class$componentType = null
  | static $class$ifTable = java.lang.Object[0]@1897064952 (0x7112e9f8)
  | static $class$classFlags = 32
  | static $class$extData = null
  | static $class$shadow$monitor = 536870912
  | static $class$copiedMethodsOffset = 2
  | static $class$status = 11
  | static $class$superClass = dalvik.system.BaseDexClassLoader
  | pathList = dalvik.system.DexPathList@315228296 (0x12ca0088)
  | allocator = 505980096432
  | classTable = 505980941184
  | packages = java.util.HashMap@315228216 (0x12ca0038)
  | parent = java.lang.BootClassLoader@1897088160 (0x711344a0)
  | proxyCache = java.util.HashMap@315228256 (0x12ca0060)
  | runtimeInternalObjects = java.lang.Object[247]@315150700 (0x12c8d16c)
  | shadow$klass = dalvik.system.PathClassLoader
  | shadow$monitor = 0
• Array of java.lang.Object[]
  | [0] = com.squareup.leakcanary.DefaultLeakDirectoryProvider$1
  | [1] = com.squareup.leakcanary.DefaultLeakDirectoryProvider$3
  | [2] = com.squareup.leakcanary.internal.FutureResult
  | [3] = com.squareup.leakcanary.AndroidHeapDumper$1
  | [4] = com.squareup.leakcanary.GcTrigger$1
  | [5] = com.squareup.leakcanary.RefWatcher
  | [6] = com.squareup.leakcanary.RefWatcher$1
  | [7] = com.squareup.leakcanary.AndroidWatchExecutor$3
  | [8] = com.squareup.leakcanary.ExcludedRefs$ParamsBuilder
  | [9] = com.squareup.leakcanary.Retryable$Result[]
  | [10] = com.karumi.dexter.Dexter
  | [11] = com.squareup.leakcanary.internal.LeakCanarySingleThreadFactory
  | [12] = com.karumi.dexter.listener.PermissionRequest
  | [13] = com.squareup.leakcanary.ExcludedRefs
  | [14] = com.karumi.dexter.DexterBuilder
  | [15] = com.karumi.dexter.AndroidPermissionService
  | [16] = com.karumi.dexter.listener.single.SnackbarOnDeniedPermissionListener
  | [17] = com.squareup.leakcanary.internal.LeakCanaryInternals$1
  | [18] = com.karumi.dexter.sample.SampleBackgroundThreadPermissionListener
  | [19] = com.karumi.dexter.sample.SampleMultiplePermissionListener
  | [20] = butterknife.Unbinder
  | [21] = com.squareup.leakcanary.AndroidExcludedRefs$40
  | [22] = com.karumi.dexter.PermissionRationaleToken
  | [23] = com.squareup.leakcanary.AndroidExcludedRefs$41
  | [24] = com.karumi.dexter.listener.multi.SnackbarOnAnyDeniedMultiplePermissionsListener$Builder$1
  | [25] = com.squareup.leakcanary.AndroidExcludedRefs$30
  | [26] = com.squareup.leakcanary.ServiceHeapDumpListener
  | [27] = com.squareup.leakcanary.AndroidExcludedRefs$31
  | [28] = com.squareup.leakcanary.AndroidExcludedRefs$20
  | [29] = android.support.v4.app.ActivityCompatApi23$RequestPermissionsRequestCodeValidator
  | [30] = com.karumi.dexter.DexterBuilder$Permission
  | [31] = com.squareup.leakcanary.AndroidExcludedRefs$32
  | [32] = com.squareup.leakcanary.ActivityRefWatcher
  | [33] = com.squareup.leakcanary.AndroidExcludedRefs$21
  | [34] = com.karumi.dexter.Thread
  | [35] = com.squareup.leakcanary.AndroidExcludedRefs$10
  | [36] = butterknife.internal.DebouncingOnClickListener$1
  | [37] = com.karumi.dexter.MultiplePermissionListenerThreadDecorator
  | [38] = com.karumi.dexter.sample.SampleActivity_ViewBinding$1
  | [39] = com.squareup.leakcanary.AndroidExcludedRefs$33
  | [40] = com.squareup.leakcanary.AndroidExcludedRefs$22
  | [41] = com.squareup.leakcanary.AndroidExcludedRefs$11
  | [42] = com.karumi.dexter.sample.SampleActivity_ViewBinding$2
  | [43] = com.karumi.dexter.sample.SampleApplication
  | [44] = com.squareup.leakcanary.AndroidExcludedRefs$34
  | [45] = butterknife.internal.DebouncingOnClickListener
  | [46] = com.karumi.dexter.listener.single.PermissionListener[]
  | [47] = com.karumi.dexter.listener.single.BasePermissionListener
  | [48] = com.squareup.leakcanary.AndroidExcludedRefs$23
  | [49] = com.squareup.leakcanary.LeakCanary
  | [50] = com.squareup.leakcanary.AndroidExcludedRefs$12
  | [51] = com.karumi.dexter.listener.PermissionRequestErrorListener
  | [52] = com.squareup.leakcanary.HeapDump$Listener$1
  | [53] = com.karumi.dexter.sample.SampleActivity_ViewBinding$3
  | [54] = com.squareup.leakcanary.AndroidExcludedRefs$35
  | [55] = com.squareup.leakcanary.AndroidExcludedRefs
  | [56] = com.squareup.leakcanary.AndroidExcludedRefs$24
  | [57] = com.squareup.leakcanary.AndroidExcludedRefs$13
  | [58] = com.karumi.dexter.DexterBuilder$SinglePermissionListener
  | [59] = com.squareup.leakcanary.GcTrigger
  | [60] = com.karumi.dexter.listener.multi.MultiplePermissionsListener
  | [61] = com.karumi.dexter.sample.SampleActivity_ViewBinding$4
  | [62] = com.squareup.leakcanary.AndroidExcludedRefs$36
  | [63] = com.squareup.leakcanary.AndroidExcludedRefs$25
  | [64] = android.support.v4.app.ActivityCompat
  | [65] = com.squareup.leakcanary.AndroidExcludedRefs$14
  | [66] = android.support.v4.content.PermissionChecker
  | [67] = com.squareup.leakcanary.AndroidExcludedRefs$37
  | [68] = com.squareup.leakcanary.ExcludedRefs$Builder
  | [69] = com.squareup.leakcanary.AndroidExcludedRefs$26
  | [70] = com.squareup.leakcanary.AndroidExcludedRefs$15
  | [71] = com.squareup.leakcanary.RefWatcherBuilder
  | [72] = com.squareup.leakcanary.AndroidExcludedRefs$38
  | [73] = com.squareup.leakcanary.AndroidExcludedRefs$27
  | [74] = com.squareup.leakcanary.AndroidExcludedRefs$16
  | [75] = com.squareup.leakcanary.AndroidExcludedRefs$39
  | [76] = com.squareup.leakcanary.AndroidExcludedRefs$28
  | [77] = com.squareup.leakcanary.AndroidExcludedRefs$17
  | [78] = com.squareup.leakcanary.AndroidExcludedRefs$1
  | [79] = android.support.design.widget.Snackbar$Callback
  | [80] = com.karumi.dexter.DexterBuilder$MultiPermissionListener
  | [81] = butterknife.ButterKnife
  | [82] = android.support.v4.app.ActivityCompatApi23
  | [83] = com.squareup.leakcanary.AndroidExcludedRefs$29
  | [84] = com.squareup.leakcanary.AndroidExcludedRefs$18
  | [85] = com.squareup.leakcanary.AndroidExcludedRefs$2
  | [86] = com.squareup.leakcanary.Retryable$Result
  | [87] = com.karumi.dexter.listener.single.DialogOnDeniedPermissionListener$Builder
  | [88] = com.squareup.leakcanary.ActivityRefWatcher$1
  | [89] = com.karumi.dexter.sample.SamplePermissionListener
  | [90] = com.squareup.leakcanary.AndroidExcludedRefs$19
  | [91] = com.squareup.leakcanary.AndroidExcludedRefs$3
  | [92] = butterknife.internal.Utils
  | [93] = com.squareup.leakcanary.AndroidWatchExecutor$2
  | [94] = com.karumi.dexter.listener.multi.BaseMultiplePermissionsListener
  | [95] = com.squareup.leakcanary.WatchExecutor
  | [96] = com.squareup.leakcanary.AndroidExcludedRefs$4
  | [97] = com.karumi.dexter.sample.SampleActivity_ViewBinding
  | [98] = com.karumi.dexter.listener.multi.CompositeMultiplePermissionsListener
  | [99] = com.squareup.leakcanary.HeapDumper
  | [100] = com.karumi.dexter.listener.single.PermissionListener
  | [101] = com.squareup.leakcanary.AndroidExcludedRefs$5
  | [102] = com.karumi.dexter.listener.multi.SnackbarOnAnyDeniedMultiplePermissionsListener$Builder
  | [103] = com.squareup.leakcanary.KeyedWeakReference
  | [104] = com.karumi.dexter.sample.SampleErrorListener
  | [105] = com.squareup.leakcanary.AndroidExcludedRefs$6
  | [106] = android.support.design.widget.BaseTransientBottomBar$BaseCallback
  | [107] = com.squareup.leakcanary.AndroidWatchExecutor
  | [108] = com.squareup.leakcanary.DebuggerControl
  | [109] = com.karumi.dexter.ThreadFactory
  | [110] = com.squareup.leakcanary.internal.LeakCanaryInternals
  | [111] = android.support.v4.app.ActivityCompat$OnRequestPermissionsResultCallback
  | [112] = com.squareup.leakcanary.AndroidExcludedRefs$7
  | [113] = com.squareup.leakcanary.internal.HeapAnalyzerService
  | [114] = com.squareup.leakcanary.Preconditions
  | [115] = android.support.v4.content.ContextCompat
  | [116] = com.karumi.dexter.DexterInstance
  | [117] = com.squareup.leakcanary.AndroidExcludedRefs$8
  | [118] = com.squareup.leakcanary.Retryable
  | [119] = com.squareup.leakcanary.AndroidExcludedRefs[]
  | [120] = com.karumi.dexter.DexterException
  | [121] = com.squareup.leakcanary.AndroidExcludedRefs$9
  | [122] = com.squareup.leakcanary.LeakDirectoryProvider
  | [123] = com.squareup.leakcanary.DebuggerControl$1
  | [124] = com.squareup.leakcanary.DisplayLeakService
  | [125] = com.karumi.dexter.MultiplePermiss...

[sklinefelter]

@Serchinastico is there anything else that you need on my end?

[wesley-firewalla]

is there any updates for this memory leak issue?

[Serchinastico]

Not really, I tested it several times but results weren't consistent and I want to make sure we get rid of the issue for once

[Serchinastico]

Ok, so it looks like there is a leak in onActivityDestroyed as well, I'm going to:

• Recreate the PR fixing the broken test
• Clean the listener reference there too
• Merge the PR.

I'm including some other pending PRs with non backwards compatible changes to next version so expect to see these changes in v5.0.0

Thank you all for your feedback and collaboration 👏

[Serchinastico]

Continues in #210