Requesting and checking permissions in parallel not working
norman-kapschefsky opened this issue · 11 comments
Description
Hi, I think I found a race condition between checking and requesting permissions.
I have the following setup:
- FragmentA and FragmentB are open in parallel within tabs
- FragmentA is requesting permissions -> system dialog appears (not selected anything yet)
- While dialog is displayed (from FragmentA), FragmentB checks for permissions within onMapReady() callback
- When I select something on the permission dialog, I wont get the result back in FragmentA anymore.
In my opinion is the reason here in the RuntimePermissionRequest
init block. It overwrites the listeners in ResultLauncherRuntimePermissionHandler
. I know you described in ResultLauncherRuntimePermissionHandler
that it can only handle one request at a time, but I think a simple check for permissions should act in a different way.
What do you think about this behavior? Thank you in advance.
KPermissions version:
3.3.0
API level:
33
I saw that you provide Activity.checkPermissionsStatus
extensions without using any Request
objects for it. I will use it instead. But maybe other people running into the same problem mentioned above. I think it could be a design weakness.
Thanks for the feedback. I'll investigate this behaviour to check if this library can support multiple permissions request simultaneously.
Just to be clear, correct me if I'm wrong: in your case, two permissions dialogs are on screen and when the second one is dismissed, every action happening on the first one is ignored right?
Hi @fondesa, thanks for having a look.
Actually its a different case:
- I have requested a permission where the dialog gets displayed (stays open)
- In addition, there is a delayed (parallel task/screen/whatever) that is checking for permissions with
PermissionRequestBuilder.checkStatus()
Afterwards the permission result from the dialog is not processed anymore, because the checkStatus()
has overwritten things internally.
Thanks, I'll try to take a look into this case!
I couldn't reproduce this.
I've tried the following setup:
- Both FragmentA and FragmentB create a different instance of permission requests
- FragmentA calls
send()
immediately and the permission dialog is shown - FragmentB waits 3 second while the permission dialog is still on screen and calls
checkStatus()
- I accept the permissions in the dialog
- I receive the permission result from the dialog with the right
PermissionStatus
Since it works in my case, I'll ask you other questions to try to reproduce it (unless you want to provide a sample project to reproduce it):
- Are you creating 2 DIFFERENT instances of permission requests?
- If so, do they contain different permissions or the same ones?
- How do you interact with the permission dialog? Do you just accept them after
onMapReady()
?
Hi @fondesa,
thanks for your effort so far. I wrote a litte sample of how you can reproduce it. I also did a short video out of it:
https://user-images.githubusercontent.com/813760/201675286-7c6d81cd-b317-4b8b-81c9-e84923466763.mov
Here is also the code in a written way:
import android.Manifest
import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.widget.Button
import android.widget.FrameLayout
import androidx.fragment.app.Fragment
import androidx.lifecycle.lifecycleScope
import com.fondesa.kpermissions.extension.permissionsBuilder
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
class PermissionManagerTestFragment : Fragment() {
override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? =
FrameLayout(inflater.context).apply {
layoutParams = FrameLayout.LayoutParams(FrameLayout.LayoutParams.MATCH_PARENT, FrameLayout.LayoutParams.MATCH_PARENT)
addView(
Button(inflater.context).apply {
text = "Request permission"
layoutParams = FrameLayout.LayoutParams(FrameLayout.LayoutParams.MATCH_PARENT, FrameLayout.LayoutParams.WRAP_CONTENT)
setOnClickListener {
val permission = Manifest.permission.ACCESS_FINE_LOCATION
// request permission dialog
println("!!!! Request permission Dialog...")
requireActivity().permissionsBuilder(permission)
.build()
.apply { addListener { result -> println("!!!! Dialog permission result: $result") } }
.send()
// check permission status
viewLifecycleOwner.lifecycleScope.launch {
delay(500L)
val result = requireActivity().permissionsBuilder(permission).build().checkStatus()
println("!!!! Check permission status: $result")
}
// result in "addListener" is never called if checkStatus were executed
}
}
)
}
}
The delay btw doesn't matter. I just wanted to demo our case with the "onMapReady()" callback. I think the code also answers your questions.
Hope this helps to understand my perspective in a better way :)
Oh yes, the problem here is that you are creating multiple instances of the same permission request.
If you build a single permission request, you can call both send()
and checkStatus()
without problems.
Said that, even if not recommended at all, I should be able to handle this case where multiple instances of PermissionRequests are created so I'm leaving this open, hopefully your problem is solved though.
Two additional notes not strictly related to your issue:
- You don't need to call
requireActivity()
since there's also a builder for fragments, you can call directlypermissionsBuilder()
- Be careful about your implementation with
addListener { }
, in that way, you'll add a new listener every time you callsetOnClickListener()
. I suggest you to calladdListener { }
outside ofsetOnClickListener()
Thanks for your quick reply. Maybe it's better to get rid of the checkStatus
method entirely from the request and just use it for "requesting permissions" only. Since you provide static Activity.checkPermissionsStatus(...)
function to obtain the state, it's fine and the other approach is kind of redundant.
But anyway, thanks so far. Feel free to close the issue at any time on your own.
Thanks for your quick reply. Maybe it's better to get rid of the checkStatus method entirely from the request and just use it for "requesting permissions" only. Since you provide static Activity.checkPermissionsStatus(...) function to obtain the state, it's fine and the other approach is kind of redundant.
I added checkStatus()
on PermissionRequest
for dependency injection reasons. In this way, users can inject fake/mock PermissionRequest
instances in their tests. Providing only the static method would make this behavior unachievable (unless the user create an abstraction over the library as well).
Marking this as done since the PR should fix the original issue. Feel free to re-open this if necessary.
Thanks for your input and time to open the issue.