canelmas / let

Annotation based simple API flavored with AOP to handle new Android runtime permission model
Apache License 2.0
530 stars 40 forks source link

Leaked Activity instance #13

Open jbialkowski13 opened 8 years ago

jbialkowski13 commented 8 years ago

Hi,

Thanks for great library which really makes it easier working with runtime permissions.

However I have discovered really big leak using that library. It might be similar to already reported issue #12.

The problem is directly connected to killing activity and recreating state by Android OS. (For making that easy reproducible enable "Do not keep activities" in Developer settings.)

Really simple code for reproducing mentioned issue:

public class MainActivity extends AppCompatActivity implements RuntimePermissionListener {

    private static final String TAG = MainActivity.class.getSimpleName();

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
        Log.i(TAG, "Created " + this.hashCode());
        final Button cameraButton = (Button) findViewById(R.id.camera);
        cameraButton.setOnClickListener(new OnClickListener() {
            @Override
            public void onClick(View view) {
                Log.i(TAG, "Access prompted for " + MainActivity.this.hashCode());
                someMethodWhichUsesCamera();
            }
        });
    }

    @AskPermission({Manifest.permission.CAMERA})
    private void someMethodWhichUsesCamera() {
        Log.i(TAG, "Camera accessible for " + this.hashCode());
    }

    @Override
    public void onShowPermissionRationale(List<String> permissionList, RuntimePermissionRequest permissionRequest) {
        //do nothing
    }

    @Override
    public void onPermissionDenied(List<DeniedPermission> deniedPermissionList) {
        //do nothing
    }

    @Override
    public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) {
        Let.handle(this, requestCode, permissions, grantResults);
    }
}

As you can this code does nothing, but important thing is the hashCode value printed in logs.

For normal flow result is following:

sample.android.let.activityleak I/MainActivity: Created 48167139 sample.android.let.activityleak I/MainActivity: Access prompted for 48167139 sample.android.let.activityleak I/MainActivity: Camera accessible for 48167139

By normal flow I mean: user clicks button which asks for permission and permission is granted immediately.

That flow is in 99% correct, but imagine following scenario: (with "Do not keep activities")

  1. User clicks button
  2. User is asked for permission
  3. User puts app to background when system dialog is displayed 4 User restores application
  4. User clicks "Allow"

The output for such scheme is:

sample.android.let.activityleak I/MainActivity: Created 190272224 sample.android.let.activityleak I/MainActivity: Access prompted for 190272224 sample.android.let.activityleak I/MainActivity: Created 125354589 sample.android.let.activityleak I/MainActivity: Camera accessible for 190272224

Let is calling someMethodWhichUsesCamera on killed Activity instance, thus besides this leak other exception may arrive like NPE.

Thanks

canelmas commented 8 years ago

thanks @whiter13 for the details. I'll look into this asap.

jbialkowski13 commented 8 years ago

Hi @canelmas What is a status of issue?

canelmas commented 8 years ago

hi @whiter13, still trying to figure out a way to solve the leakage.