Fix potential crash in connection state flow

[Pururun]

I am not sure this happens in the actual app, but it is good to prevent future issues.

---

This change is 

[linear]

DROID-1224 Fix potential crash in connection state function

[Rawa]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @Pururun)

---

android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/util/ManagedChannel.kt line 26 at r1 (raw file):

        }

        awaitClose { cancel() }

I don't think this should be necessary? awaitClose block is called when the flow is cancelled, I believe this block should rather be to used to remove listeners etc.

I believe the problem we faced is that we don't have a way to unregister the notifyWhenStateChanged callback. Thus it would invoke it.resume(...) and possibly send(currentState) on a already cancelled flow.

So I believe the correct solution would be something along the lines of:

internal fun ManagedChannel.connectivityFlow(): Flow<ConnectivityState> {
    return callbackFlow {
        var currentState = getState(false)

        while (isActive) {
            // Check that we are active before sending
            send(currentState)
            currentState = suspendCancellableCoroutine {
                notifyWhenStateChanged(currentState) {
                    // If we are cancelled we don't try and invoke the it.resume
                    if (it.isActive) {
                        it.resume(getState(false))
                    }
                }
            }
        }
    }
}

Alternatively I believe the callback could possibly be, where lambda is the onCancellation callback:

                        it.resume(getState(false), { _ -> })

[Rawa]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @Pururun)

---

android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/util/ManagedChannel.kt line 26 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

I don't think this should be necessary? awaitClose block is called when the flow is cancelled, I believe this block should rather be to used to remove listeners etc.

I believe the problem we faced is that we don't have a way to unregister the notifyWhenStateChanged callback. Thus it would invoke it.resume(...) and possibly send(currentState) on a already cancelled flow.

So I believe the correct solution would be something along the lines of:

internal fun ManagedChannel.connectivityFlow(): Flow<ConnectivityState> {
    return callbackFlow {
        var currentState = getState(false)

        while (isActive) {
            // Check that we are active before sending
            send(currentState)
            currentState = suspendCancellableCoroutine {
                notifyWhenStateChanged(currentState) {
                    // If we are cancelled we don't try and invoke the it.resume
                    if (it.isActive) {
                        it.resume(getState(false))
                    }
                }
            }
        }
    }
}

Alternatively I believe the callback could possibly be, where lambda is the onCancellation callback:

                        it.resume(getState(false), { _ -> })

Clarification: With second alternative I don't believe the if (it.isActive) is needed.

[Rawa]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Pururun)

[Rawa]

Reviewable status: complete! all files reviewed, all discussions resolved

[albin-mullvad]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved