https://kotlinlang.org logo
Join Slack
Powered by
So this definitely looks wrong, and raises the "In...
# coroutines
t
So this definitely looks wrong, and raises the "Inappropriate blocking method call" warning, but it works. Is this actually dangerous?
Copy code
lifecycleScope.launch {
            repeatOnLifecycle(Lifecycle.State.STARTED) {
                try {
                    session.load()
                    awaitCancellation()
                } finally {
                    runBlocking {
                        // This is a suspending function that performs I/O and needs to complete.
                        session.recordBackgroundTime()
                    }
                }
            }
        }
The goal here is to write a value to disk when the lifecycle exits the STARTED state.
u
You are blocking the coroutine. Is that what you want? What exactly is it that should wait for the io?
t
If I don't block it, the surrounding 
lifecycleScope
is canceled and the I/O coroutine inside 
session.recordBackgroundTime
does not run.
Would GlobalScope.launch make sense here?
u
Then don't put it on lifecycle scope
t
Sure, I was just hoping to avoid a 
lateinit var
or nullable property by using 
awaitCancellation()
in the lifecycle scope
n
u
Just launch on a new scope which does not get cancelled
t
@Nick Allen Thanks!
u
What should await cancellation
t
the 
repeatOnLifecycle
block is cancelled when the lifecycle exits the STARTED state (it is stopped)
u
Just call CoroutineScope(...).launch{session.recordBackgroundTime() }
Would that do what you want?
t
Yes, but 
withContext(NonCancellable)
is working and feels like the correct thing to do here.
n
Be careful launching work into some non-child scope. It's easy to run into issues like invokeOnCompletion code that doesn't wait for all the work (because some work is not part of that scope).
e
does your cleanup do the right thing if the lifecycle re-enters STARTED while your STOPPED handler is running?
u
True. But we exactly want that work not to be part of the scope I guess
t
@ephemient I don't think that's possible with 
NonCancellable
, unless I'm not understanding what happens here.
I'll try breaking it by adding a delay
e
actually I didn't see the `runBlocking`… that is dangerous inside of coroutine context, will deadlock if you have anything that wants to 
withContext(Main)
if you can tolerate asynchronicity, this is a case where 
GlobalScope.launch {}
might be a reasonable, but should also be used with caution
t
@ephemient Interesting; adding a 5 second delay inside 
NonCancellable
(and presumably with GlobalScope) the handler ends up running after re-starting the activity
So I'll need to serialize or lock if I care about this edge case
e
or somehow cancel, which itself is tricky. yes
u
@ephemient you warn ofa new scope and suggest to use GlobalScope?
t
That would allow cancellation if I hold onto the Job, yeah
u
Same for a custom scope
t
but the session timeout is 2 minutes so racing on the recorded time is not an issue (unless a write takes longer than that)
e
@uli I mean that GlobalScope may be "desired" if the cleanup should not be cancelled, but care must be taken in all cases
u
What would GlobalScope improve over s custom scope?
t
what parent would the custom scope have though?
if the parent is the lifecycle, it will be cancelled before the write succeeds
u
Sorry that was @Nick Allen
Warning of a custom scope.
Any way, it is preferable to use CoroutineScope(...) over GlobalScope. But the idea is basically the same
n
Curious, would you rather cancel the previous recordBackgroundTime action or cancel the "new" recordBackgroundTime action?
e
CoroutineScope which gets lost when the containing object is destroyed is not better than GlobalScope if you need the coroutine to keep running
whether that's the case or not depends on usage, though
u
True. You might want to hold on to it
t
@Nick Allen I would cancel the previous job, but it's moot in this case because the logic will default to a "locked" session if we race on 
recordBackgroundTime
n
"CoroutineScope(...) over GlobalScope." <- I also disagree. Need to be careful with either.
t
also if writing to disk takes 2 minutes then this app will have other problems 🙂
u
With a property Val cleanupScope = CoroutineScope(SupervisorJob) (Excuse the layout. I am on mobile)
e
a property is also not helpful if the containing object gets destroyed
u
True again.
Then repository, singleton, application property, .... But any solution that allows to handle exceptions
As a habit
g
Copy code
> // This is a suspending function that performs I/O and needs to complete.
If your function is suspend, there is no reason to warp it to runBlocking, you block your main thread, just remove runBlocking and it will work
t
That's not correct; since this is happening after 
awaitCancellation()
, the function is called, suspended (as it uses 
withContext(<http://Dispatchers.IO|Dispatchers.IO>)
internally), and the parent scope is cancelled before the function performs the save.
g
Ah, yes, I see now, await cancellation
Probably better would be avoid blocking, and make recordBackgroundTime non-suspend, which returns immediately and under the hood has own scope to do all required io Otherwise blocking thread + delaying cancellation doesn't look right, though it may be not an issue on practice in particular case, but may cause issues later (because of blocked Main thread even cause deadlock)
Open in Slack
PreviousNext
Powered by