Reputation: 45
I have a code something like the following in my app
class MyFragment : Fragment(), CoroutineScope by MainScope() {
override fun onDestroy() {
cancel()
super.onDestroy()
}
override fun onActivityCreated(savedInstanceState: Bundle?) {
super.onActivityCreated(savedInstanceState)
doSomething()
}
private fun doSomething() = launch {
val data = withContext(Dispathers.IO) {
getData()
}
val pref = context!!.getSharedPreferences("mypref", MODE_PRIVATE)
pref.edit().putBoolean("done", true).apply()
}
}
In production, I get many NPEs in doSomething() while accessing context.
My assumption was that the coroutine gets cancelled after calling cancel() in onDestroy(), so I didn't bother to check the context for null value. But it looks like coroutine continues to execute even after cancel() is called. I think it happens if the cancel() is called after completing the withContext and before resuming the coroutine.
So I replaced doSomething() with following.
private fun doSomething() = launch {
val data = withContext(Dispathers.IO) {
getData()
}
if (isActive) {
val pref = context!!.getSharedPreferences("mypref", MODE_PRIVATE)
pref.edit().putBoolean("done", true).apply()
}
}
This fixes the crash.
However, Is this an expected behaviour or am I doing something wrong? Kotlin documentation aren't very clear about this. And most of the examples online are like my original code.
Upvotes: 4
Views: 4675
Reputation: 40522
Your code assumes that the withContext()
will stop execution when it returns, if the scope is cancelled, but actually it didn't, till version 1.3.0 of kotlin coroutines. Here is GitHub issue. I guess that you are using an earlier version of the library.
I also recommend you use LifecycleScope instead of a custom scope. It's part of lifecycle-runtime-ktx
library. So, simplified solution looks like:
// build.gradle
dependencies {
...
implementation "androidx.lifecycle:lifecycle-runtime-ktx:2.2.0-rc02"
}
class MyFragment : Fragment() {
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
doSomething()
}
private fun doSomething() = viewLifecycleOwner.lifecycleScope.launch {
val data = withContext(Dispathers.IO) {
getData()
}
val pref = context!!.getSharedPreferences("mypref", MODE_PRIVATE)
pref.edit().putBoolean("done", true).apply()
}
}
There are other helpful utilities which simplifies coroutines usage, take a look at Use Kotlin coroutines with Architecture components documentation section.
Upvotes: 7
Reputation: 812
The latest coroutine updates have introduced the possibility of working safe with scopes. Basically if you are doing async work in Fragments or Activities you use lifecycleScope
and if you are going async work in ViewModel you need to use viewModelScope
. There is also a GlobalScope that can be used in any class but it was proved to have many stability flaws and should be avoided. In any case scenario you should use either lifecycleScope or viewModelScope and use suspend functions in other classes.
So here is the way to do it:
override fun onActivityCreated(savedInstanceState: Bundle?) {
super.onActivityCreated(savedInstanceState)
lifecycleScope.launch {
doSomething()
}
}
private suspend fun doSomething(){
withContext(Dispatchers.IO){
getData()
if (isActive) {
val pref = context!!.getSharedPreferences("mypref", MODE_PRIVATE)
pref.edit().putBoolean("done", true).apply()
}
}
}
As you can see in the code above, there is no need for a scope or launch in the doSomething function because that acts as a task that gets executed in the already launched coroutine above in onActivityCreated. Launching a new coroutine in the doSomething() function is pointless. Also, you can encapsulate your entire code in your withContext() statement because getting shared preferences is also a transaction that works best on Dispatchers.IO. Alternatively you can change the context to the desired context anytime you want, like:
withContext(Dispatchers.IO){
getData()
if (isActive) {
val pref = context!!.getSharedPreferences("mypref", MODE_PRIVATE)
pref.edit().putBoolean("done", true).apply()
}
withContext(Dispatchers.Default){
// Heavy computing
}
}
Always remember that Dispatchers.IO is optimised best for database transactions, shared preferences and networking calls. If you do heavy computing like forEach in forEach and stuff like that, use Dispatchers.Default and if you need to do view work, use Dispatchers.Main.
These scopes make sure your task gets executed without interruption from other activity components, and if it is interrupted by something like activity destruction, it prevents exceptions and uses garbage collector properly on everything that's inside the scope. Since I started using the scopes, there were no more crashes for me.
Upvotes: 0
Reputation: 2560
This is the expected behaviour since Coroutine Cancellation is cooperative in nature.
While suspend functions will check for the state of coroutine in your case you will have to do this yourself.
A cleaner way is to have these operations in a viewmodel and just cancel the coroutine in ViewModel onCleared
; if you are using ViewModel
Upvotes: 1