[ktx] Construct PermissionsRequester from Fragment may cause that callbacks (requiresPermission, onPermissionDenied, etc.) are called after Fragment instance is already dead

omtians9425 commented 2 years ago

Overview

When we open and close the same Fragment that constructs PermissionRequester multiple times the callback passed by closed one's instance is called and this may cause a crash.

Reproducible steps

In my sample app,
- Open app then FirstFragment is launched as a root Fragment
- Navigate to SecondFragment (PermissionsRequester is constructed here) with tapping "CLICK TO NEXT"
- Back to FirstFragment
- Navigate to SecondFragment again
- Tap "REQUEST PERMISSION"
- Tap "DENY" or "ALLOW"
- App crashes (Toast is about to be shown in Fragment's Context with a permission result callback)

Stack trace:

Toast uses detached Context so java.lang.IllegalStateException: Fragment SecondFragment{9e09ebc} (c9cd8d4c-7acc-476a-90c8-7e882fc8dd69) not attached to a context. is shown

2022-02-25 01:27:24.555 21578-21578/com.example.omtians9425.permissionsdispatcherktxsample E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.example.omtians9425.permissionsdispatcherktxsample, PID: 21578
    java.lang.RuntimeException: Failure delivering result ResultInfo{who=@android:requestPermissions:, request=1462789988, result=-1, data=Intent { act=android.content.pm.action.REQUEST_PERMISSIONS (has extras) }} to activity {com.example.omtians9425.permissionsdispatcherktxsample/com.example.omtians9425.permissionsdispatcherktxsample.MainActivity}: java.lang.IllegalStateException: Fragment SecondFragment{9e09ebc} (c9cd8d4c-7acc-476a-90c8-7e882fc8dd69) not attached to a context.
        at android.app.ActivityThread.deliverResults(ActivityThread.java:4360)
        at android.app.ActivityThread.handleSendResult(ActivityThread.java:4402)
        at android.app.servertransaction.ActivityResultItem.execute(ActivityResultItem.java:49)
        at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:108)
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:68)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1808)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:6669)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)
     Caused by: java.lang.IllegalStateException: Fragment SecondFragment{9e09ebc} (c9cd8d4c-7acc-476a-90c8-7e882fc8dd69) not attached to a context.
        at androidx.fragment.app.Fragment.requireContext(Fragment.java:919)
        at com.example.omtians9425.permissionsdispatcherktxsample.SecondFragment$onAttach$1.invoke(SecondFragment.kt:24)
        at com.example.omtians9425.permissionsdispatcherktxsample.SecondFragment$onAttach$1.invoke(SecondFragment.kt:20)
        at permissions.dispatcher.ktx.PermissionRequestViewModel$observe$1.onChanged(PermissionRequestViewModel.kt:33)
        at permissions.dispatcher.ktx.PermissionRequestViewModel$observe$1.onChanged(PermissionRequestViewModel.kt:30)
        at androidx.lifecycle.LiveData.considerNotify(LiveData.java:133)
        at androidx.lifecycle.LiveData.dispatchingValue(LiveData.java:151)
        at androidx.lifecycle.LiveData.setValue(LiveData.java:309)
        at androidx.lifecycle.MutableLiveData.setValue(MutableLiveData.java:50)
        at permissions.dispatcher.ktx.PermissionRequestViewModel.notifyObserver(PermissionRequestViewModel.kt:43)
        at permissions.dispatcher.ktx.PermissionRequestViewModel.postPermissionRequestResult(PermissionRequestViewModel.kt:20)
        at permissions.dispatcher.ktx.PermissionRequestFragment$NormalRequestPermissionFragment.onRequestPermissionsResult(PermissionRequestFragment.kt:53)
        at androidx.fragment.app.FragmentManager$9.onActivityResult(FragmentManager.java:2679)
        at androidx.fragment.app.FragmentManager$9.onActivityResult(FragmentManager.java:2651)
        at androidx.activity.result.ActivityResultRegistry.doDispatch(ActivityResultRegistry.java:392)
        at androidx.activity.result.ActivityResultRegistry.dispatchResult(ActivityResultRegistry.java:351)
        at androidx.activity.ComponentActivity.onRequestPermissionsResult(ComponentActivity.java:667)
        at androidx.fragment.app.FragmentActivity.onRequestPermissionsResult(FragmentActivity.java:612)
        at android.app.Activity.dispatchRequestPermissionsResult(Activity.java:7608)
        at android.app.Activity.dispatchActivityResult(Activity.java:7458)
        at android.app.ActivityThread.deliverResults(ActivityThread.java:4353)
        at android.app.ActivityThread.handleSendResult(ActivityThread.java:4402) 
        at android.app.servertransaction.ActivityResultItem.execute(ActivityResultItem.java:49) 
        at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:108) 
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:68) 
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1808) 
        at android.os.Handler.dispatchMessage(Handler.java:106) 
        at android.os.Looper.loop(Looper.java:193) 
        at android.app.ActivityThread.main(ActivityThread.java:6669) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

GIF with the above procedure

Expected

Permission result callbacks passed by Fragment that is alive (most recently opened one) are correctly called without any crashes.

Actual

App crashes with permission result callbacks being called passed by Fragment that is already dead.

Environment

Which library version are you using?
- ktx:1.1.3
On which devices do you observe the issue?
- I confirmed with Pixel 2 API 28

Hypothesis for the cause of the crash

LiveData is observed with using Activity as a LifecycleOwner regardless of whether PermissionsRequster was constructed by Fragment or Activity.
If PermissionsRequster was constructed by Fragment, lambda passed into LiveData.observe will still be registered, and observing LiveData won't stop when the Fragment dies
When the same Fragment is reopened and the LiveData is updated, the previous lambda is called

omtians9425 commented 2 years ago

@hotchemi I'd appreciate it if you could take a look when you have time (There is no rush) 🙇

nguyennh-0786 commented 2 years ago

@hotchemi how about this PR?

ColorfulHorse commented 2 years ago

@hotchemi I'd appreciate it if you could take a look when you have time (There is no rush) 🙇

maybe lifecycleOwner = this is not always right，fragment lifecycle is different from fragment viewLifecycle

Oussemahlel commented 2 years ago

I have the same problem, any solution for this bug!!!

permissions-dispatcher / PermissionsDispatcher