https://kotlinlang.org logo
Title
t

tad

07/29/2021, 7:42 PM
So this definitely looks wrong, and raises the "Inappropriate blocking method call" warning, but it works. Is this actually dangerous?
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

uli

07/29/2021, 7:49 PM
You are blocking the coroutine. Is that what you want? What exactly is it that should wait for the io?
t

tad

07/29/2021, 7:51 PM
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

uli

07/29/2021, 7:53 PM
Then don't put it on lifecycle scope
t

tad

07/29/2021, 7:54 PM
Sure, I was just hoping to avoid a
lateinit var
or nullable property by using
awaitCancellation()
in the lifecycle scope
u

uli

07/29/2021, 7:55 PM
Just launch on a new scope which does not get cancelled
t

tad

07/29/2021, 7:56 PM
@Nick Allen Thanks!
u

uli

07/29/2021, 7:56 PM
What should await cancellation
t

tad

07/29/2021, 7:57 PM
the
repeatOnLifecycle
block is cancelled when the lifecycle exits the STARTED state (it is stopped)
u

uli

07/29/2021, 8:01 PM
Just call CoroutineScope(...).launch{session.recordBackgroundTime() }
Would that do what you want?
t

tad

07/29/2021, 8:03 PM
Yes, but
withContext(NonCancellable)
is working and feels like the correct thing to do here.
n

Nick Allen

07/29/2021, 8:05 PM
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

ephemient

07/29/2021, 8:08 PM
does your cleanup do the right thing if the lifecycle re-enters STARTED while your STOPPED handler is running?
u

uli

07/29/2021, 8:09 PM
True. But we exactly want that work not to be part of the scope I guess
t

tad

07/29/2021, 8:09 PM
@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

ephemient

07/29/2021, 8:12 PM
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

tad

07/29/2021, 8:15 PM
@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

ephemient

07/29/2021, 8:16 PM
or somehow cancel, which itself is tricky. yes
u

uli

07/29/2021, 8:18 PM
@ephemient you warn ofa new scope and suggest to use GlobalScope?
t

tad

07/29/2021, 8:19 PM
That would allow cancellation if I hold onto the Job, yeah
u

uli

07/29/2021, 8:20 PM
Same for a custom scope
t

tad

07/29/2021, 8:20 PM
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

ephemient

07/29/2021, 8:20 PM
@uli I mean that GlobalScope may be "desired" if the cleanup should not be cancelled, but care must be taken in all cases
u

uli

07/29/2021, 8:21 PM
What would GlobalScope improve over s custom scope?
t

tad

07/29/2021, 8:21 PM
what parent would the custom scope have though?
if the parent is the lifecycle, it will be cancelled before the write succeeds
u

uli

07/29/2021, 8:21 PM
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

Nick Allen

07/29/2021, 8:23 PM
Curious, would you rather cancel the previous recordBackgroundTime action or cancel the "new" recordBackgroundTime action?
e

ephemient

07/29/2021, 8:23 PM
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

uli

07/29/2021, 8:24 PM
True. You might want to hold on to it
t

tad

07/29/2021, 8:24 PM
@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

Nick Allen

07/29/2021, 8:24 PM
"CoroutineScope(...) over GlobalScope." <- I also disagree. Need to be careful with either.
t

tad

07/29/2021, 8:25 PM
also if writing to disk takes 2 minutes then this app will have other problems 🙂
u

uli

07/29/2021, 8:27 PM
With a property Val cleanupScope = CoroutineScope(SupervisorJob) (Excuse the layout. I am on mobile)
e

ephemient

07/29/2021, 8:27 PM
a property is also not helpful if the containing object gets destroyed
u

uli

07/29/2021, 8:31 PM
True again.
Then repository, singleton, application property, .... But any solution that allows to handle exceptions
As a habit
g

gildor

07/30/2021, 2:41 AM
> // 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

tad

07/30/2021, 7:55 PM
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

gildor

08/01/2021, 3:53 AM
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)